Changing Review Request Ownership

Review Request #6990 — Created Feb. 27, 2015 and submitted

Information

Review Board
master
05c1791...

Reviewers

make the ownership changeable on the UI level

Main code describtions:

In builtin_fields.py we change the 'submitter' field to inherit BaseEditableField so that the 'submitter' field will be editable; The override function--record_change_entry disposes what will be showed in change description; Then override the function to render the only one value for a change description string to HTML instead of a list.

In changedescs/models.py we describe the 'new' and 'old' changes in 'submitter' field and render the value to HTML.

Changes in models/review_request_draft.py are to add 'owner' field to override 'submitter' field. Since 'submitter' field is a property attribute of draftreviewrequest, if we change the'submitter' field from property to attribute directly, 'submitter' of older data in the dababase will be null. This will cause troubles.

Changes in reviewboard/reviews/evolutions are about Writing Database Evolutions of adding 'owner' field to the draft table.

In webapi/resources/review_request_draft.py we get the change information of 'submitter' field and set draft.owner if the change is available.

In reviewRequestEditorView.js we make the field can be autocompleted then show in proper format when updated.

In models/reviewRequestEditorModel.js we detect the changes of 'submitter' field in the front end before sending message to the back end. And show the confirm notification only when 'submitter' field was modified.

In models/draftReviewRequestModel.js: parseResourceData we set the 'submitter' info to the data from response message so that we can use it to update the field.

Passed the http://localhost:8000/js-tests/ for the front end.
Passed tests for the back end.

Added tests:

./reviewboard/manage.py test -- reviewboard.webapi.tests.test_review_request_draft:ResourceTests.test_put_publish_with_new_submitter
This test is about wether the feature works to set the submitter field.

reviewRequestEditorModelTests:
Check to set empty value.
Check if set value successfully.
Check if error thrown when set invalid value.

reviewRequestEditorViewTests:
Check confirm notification works before publish.
Check field can be autocomplete.
Check field can edit.
Check save works.
Check value format after update.
Check edit count successfully.

draftReviewRequestModelTests:
Check parse works for 'submitter' field.

Description From Last Updated

Col: 80 E501 line too long (100 > 79 characters)

reviewbotreviewbot

'model_cache' imported but unused

reviewbotreviewbot

Col: 80 E501 line too long (98 > 79 characters)

reviewbotreviewbot

Col: 21 E265 block comment should start with '# '

reviewbotreviewbot

Col: 47 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Col: 47 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 55 E261 at least two spaces before inline comment

reviewbotreviewbot

Col: 56 E262 inline comment should start with '# '

reviewbotreviewbot

Col: 9 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 21 E265 block comment should start with '# '

reviewbotreviewbot

'UserResource' imported but unused

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 22 E225 missing whitespace around operator

reviewbotreviewbot

Col: 9 E128 continuation line under-indented for visual indent

reviewbotreviewbot

I guess assocated is a typo

ChesterChester

Remove trailing commas.

VT VTL-Developer

Use single quotes

brenniebrennie

Use single quotes.

brenniebrennie

We don't support ternary operators, you can do self.owner or self.review_request.submitter

VT VTL-Developer

how about a white space before else if?

ChesterChester

Use single quotes.

brenniebrennie

I think the indentation of each argument should be consistent. Please see line 195-197

ChesterChester

Remove blank lines.

VT VTL-Developer

Use single quotes for this. The quote marks should line up. You may want to assign confirm(gettext(...)) to a value …

brenniebrennie

Should this be 'submitter' instead?

XU xuanyi

"Convert"

brenniebrennie

How about makeLink or convertToLink ?

brenniebrennie

I guess we could do this also if we do want to explictly return '' if item is null. if …

XU xuanyi

I try to stay away from variables named data because technically all variables are data :)

brenniebrennie

We do processing for other fields in this way because we can have multiple values in them. I don't think …

brenniebrennie

Blank line between these.

brenniebrennie

Col: 36 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 36 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Might be worth adding a comment about why we need to put old_value and new_value into tuples.

mike_conleymike_conley

Col: 80 E501 line too long (82 > 79 characters)

reviewbotreviewbot

We might want to add some documentation saying that the user will have the chance to cancel the publish in …

mike_conleymike_conley

I think this might be clearer: 'Are you sure you want to change the ownership of this review request? Doing …

mike_conleymike_conley

Should start with a capital letter and end in a period.

daviddavid

Undo this change (lists in python should have trailing commas)

daviddavid

This should have from __future__ import unicode_literals as the first line, and then a blank line, then the rest.

daviddavid

There will only ever be one submitter, so we can just use gettext, not ngettext.

daviddavid

These two lines can be combined (define and assign in one go)

daviddavid

You could even just move the gettext call inline here, since that text is only used once.

daviddavid

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 39 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Blank line between these.

brenniebrennie

You can put $link on the previous line. Make sure to dedent the following two lines.

brenniebrennie

You can just build it all at once using a template, e.g. _linkTemplate: _.template('<a href="<%- url %>"><%- label %></a>'), /* …

brenniebrennie

Isn't $field.children() already a jQuery wrapper around the DOM elements?

brenniebrennie

submitter before summary

brenniebrennie

submitter before summary

brenniebrennie

Blank line between these.

brenniebrennie

Can you either move the template definition into the initialize method (storing it as a variable on the object), or …

daviddavid

These should be indented 4 more spaces.

daviddavid

This is indented too far.

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/resources/models/reviewRequestModel.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/resources/models/reviewRequestModel.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. Show all issues
    Col: 80
     E501 line too long (100 > 79 characters)
    
  3. Show all issues
     'model_cache' imported but unused
    
  4. Show all issues
    Col: 80
     E501 line too long (98 > 79 characters)
    
  5. Show all issues
    Col: 21
     E265 block comment should start with '# '
    
  6. Show all issues
    Col: 47
     E127 continuation line over-indented for visual indent
    
  7. Show all issues
    Col: 47
     E127 continuation line over-indented for visual indent
    
  8. Show all issues
    Col: 13
     E265 block comment should start with '# '
    
  9. Show all issues
    Col: 55
     E261 at least two spaces before inline comment
    
  10. Show all issues
    Col: 56
     E262 inline comment should start with '# '
    
  11. 
      
Chester
  1. Could you please make the description more detailed? Because it is kind of hard to understand what you are doing in here.

  2. 
      
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/resources/models/reviewRequestModel.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/resources/models/reviewRequestModel.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. Show all issues
    Col: 9
     E128 continuation line under-indented for visual indent
    
  3. Show all issues
    Col: 21
     E265 block comment should start with '# '
    
  4. Show all issues
     'UserResource' imported but unused
    
  5. Show all issues
    Col: 13
     E265 block comment should start with '# '
    
  6. 
      
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. reviewboard/changedescs/models.py (Diff revision 3)
     
     
    Show all issues
    Col: 22
     E225 missing whitespace around operator
    
  3. Show all issues
    Col: 9
     E128 continuation line under-indented for visual indent
    
  4. 
      
CH
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
  2. Show all issues
    Col: 36
     E128 continuation line under-indented for visual indent
    
  3. Show all issues
    Col: 36
     E128 continuation line under-indented for visual indent
    
  4. 
      
Chester
  1. 
      
  2. Show all issues

    I guess assocated is a typo

  3. Show all issues

    how about a white space before else if?

  4. Show all issues

    I think the indentation of each argument should be consistent. Please see line 195-197

  5. 
      
VT
  1. 
      
  2. reviewboard/changedescs/models.py (Diff revision 4)
     
     
    Show all issues

    Remove trailing commas.

    1. Sorry, meant remove that trailing comma.

  3. Show all issues

    We don't support ternary operators, you can do self.owner or self.review_request.submitter

  4. Show all issues

    Remove blank lines.

  5. 
      
brennie
  1. 
      
  2. Show all issues

    Use single quotes

  3. Show all issues

    Use single quotes.

  4. Show all issues

    Use single quotes.

  5. Show all issues

    Use single quotes for this. The quote marks should line up.

    You may want to assign confirm(gettext(...)) to a value and check against that for readability.

    Also change "the managing privileges" to "all managing privileges."

    1. I just answered this on Slack.

      One important note is that for calls to gettext, we actually can't concatenate to wrap. In fact, we can't wrap. Otherwise, the logic for localizing will fail to see the second part of the string. So for text wrapped in gettext, we need to just let the line be as long as it needs.

  6. Show all issues

    "Convert"

  7. Show all issues

    How about makeLink or convertToLink ?

  8. Show all issues

    I try to stay away from variables named data because technically all variables are data :)

  9. Show all issues

    We do processing for other fields in this way because we can have multiple values in them. I don't think we should do this for the submitter field because we only expect one value.

    I think we should just take what we are given and try to look up exactly that.

  10. Show all issues

    Blank line between these.

  11. 
      
XU
  1. 
      
  2. Show all issues

    Should this be 'submitter' instead?

  3. reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 4)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    I guess we could do this also if we do want to explictly return '' if item is null.

    if (item) {
        str += '<a href="';
        str += (urlFunc ? urlFunc(item) : item);
        str += '">';
        str += (textFunc ? textFunc(item) : item);
        str += '</a>';
    }
    
    return str
    
  4. 
      
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
  2. Show all issues
    Col: 80
     E501 line too long (82 > 79 characters)
    
  3. 
      
mike_conley
  1. Hey chenxi - this is looking really really good. Not too many more comments frome me. Great job!

  2. reviewboard/changedescs/models.py (Diff revision 5)
     
     
     
    Show all issues

    Might be worth adding a comment about why we need to put old_value and new_value into tuples.

  3. Show all issues

    We might want to add some documentation saying that the user will have the chance to cancel the publish in the event that the submitter has been changed.

  4. Show all issues

    I think this might be clearer:

    'Are you sure you want to change the ownership of this review request? Doing so may prevent you from editing the review request afterwards.'

  5. 
      
CH
david
  1. 
      
  2. reviewboard/changedescs/models.py (Diff revision 6)
     
     
     
    Show all issues

    Should start with a capital letter and end in a period.

  3. reviewboard/changedescs/models.py (Diff revision 6)
     
     
    Show all issues

    Undo this change (lists in python should have trailing commas)

  4. Show all issues

    This should have from __future__ import unicode_literals as the first line, and then a blank line, then the rest.

  5. Show all issues

    There will only ever be one submitter, so we can just use gettext, not ngettext.

  6. Show all issues

    These two lines can be combined (define and assign in one go)

  7. Show all issues

    You could even just move the gettext call inline here, since that text is only used once.

    1. I did this before, but since the text line is too long(Christian said in gettext we shouldn't break the text into two lines) so I write in this way to avoid that. Still should I fix this?

    2. I think since that line is going to be super long no matter what, we can just have it inline and have this one be the super long line instead of moving it above.

  8. 
      
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
  2. reviewboard/changedescs/models.py (Diff revision 7)
     
     
    Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  3. Show all issues
    Col: 39
     E127 continuation line over-indented for visual indent
    
  4. 
      
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
  2. 
      
brennie
  1. This looks good. I've just got a few nitpicks.

  2. Show all issues

    Blank line between these.

    1. Can you tell we where we should leave blank line? I will remember next time.

  3. Show all issues

    You can put $link on the previous line. Make sure to dedent the following two lines.

  4. reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 8)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    You can just build it all at once using a template, e.g.

    _linkTemplate: _.template('<a href="<%- url %>"><%- label %></a>'),
    
    /*
     * ...
     */
    convertToLink: function(item, urlFunc, textFunc) {
        if (!item) {
            return '';
        }
    
        return _linkTemplate({
            url: urlFunc ? urlFunc(item) : item,
            label: textFunc ? textFunc(item) : item
        });
    
  5. Show all issues

    Isn't $field.children() already a jQuery wrapper around the DOM elements?

  6. Show all issues

    submitter before summary

  7. Show all issues

    submitter before summary

  8. Show all issues

    Blank line between these.

  9. 
      
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
  2. 
      
david
  1. 
      
  2. Show all issues

    Can you either move the template definition into the initialize method (storing it as a variable on the object), or after the if (!item) check below?

  3. Show all issues

    These should be indented 4 more spaces.

  4. Show all issues

    This is indented too far.

  5. 
      
CH
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/evolutions/__init__.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/webapi/tests/test_review_request_draft.py
        reviewboard/reviews/builtin_fields.py
        reviewboard/reviews/evolutions/add_owner_to_draft.py
        reviewboard/changedescs/models.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/tests/draftReviewRequestModelTests.js
        reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.js
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
    
    
  2. 
      
CH
Review request changed
Status:
Completed
Change Summary:
Pushed to feature/review-request-ownership (bcaa2ff)