Convert ReviewRequestEditorModel to ES6.

Review Request #9147 — Created Aug. 23, 2017 and submitted

Information

Review Board
release-3.0.x
1d50cc7...

Reviewers

Another simple conversion of code that I'm working on.

Ran js-tests.

Description From Last Updated

Missing period.

brenniebrennie

Should we instead be building an array and calling .join('')? Also: ` and "${value}"`

brenniebrennie

`"${value}"`

brenniebrennie

This should probably still pass in the function directly. No sense creating a wrapper function that does nothing but call …

chipx86chipx86

.bind(this) is implied when using this.listenTo().

chipx86chipx86

Same here.

chipx86chipx86

Same here.

chipx86chipx86

I liked using options much better, because it allows room for expansion. I'm not sure getDraftField should assume that useExtraData …

chipx86chipx86

Why the direct comparison to true?

chipx86chipx86

This could use template literals to compose the string.

chipx86chipx86

Same here.

chipx86chipx86

Sentence casing.

chipx86chipx86

This could be one line.

chipx86chipx86
brennie
  1. 
      
  2. Should we instead be building an array and calling .join('')?

    Also:

    ` and "${value}"`
    
    1. I don't think we really need to care that much about performance of building the string in this case. This gets called once in the error case.

  3. 
      
david
chipx86
  1. 
      
  2. This should probably still pass in the function directly. No sense creating a wrapper function that does nothing but call the function that listenTo would call itself.

  3. I liked using options much better, because it allows room for expansion. I'm not sure getDraftField should assume that useExtraData is the only field-related option that's going to come up (or that it should be so prominent). Can we go back to options?

  4. Why the direct comparison to true?

  5. This could use template literals to compose the string.

  6. This could be one line.

  7. 
      
david
chipx86
  1. 
      
  2. .bind(this) is implied when using this.listenTo().

  3. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (a5ae0ec)
Loading...