[WIP] Add the unreviewed section in my dashboard

Review Request #7193 — Created April 11, 2015 and discarded

Information

Review Board
master

Reviewers

Done some front-end work to add the two section to the dashboard, so far functionless, only on the web page.


 
Description From Last Updated

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 71 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 71 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 39 E231 missing whitespace after ','

reviewbotreviewbot

Col: 39 E231 missing whitespace after ','

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 39 E231 missing whitespace after ','

reviewbotreviewbot

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

reviewbotreviewbot

Col: 39 E231 missing whitespace after ','

reviewbotreviewbot

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

reviewbotreviewbot

Col: 52 E711 comparison to None should be 'if cond is None:'

reviewbotreviewbot

Col: 56 E711 comparison to None should be 'if cond is None:'

reviewbotreviewbot

Col: 56 E711 comparison to None should be 'if cond is not None:'

reviewbotreviewbot

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

reviewbotreviewbot

Use single quotes for strings.

brenniebrennie

three links? Also this should be formatted as "All", "Open", and "Unreviewed"

brenniebrennie

Blank line between these.

brenniebrennie

Blank line between these.

brenniebrennie

Blank line between these.

brenniebrennie

Undo this change.

brenniebrennie

Undo this change.

brenniebrennie

This doesn't handle the case where status is S or D so this may cause a regression.

brenniebrennie

Undo this change.

brenniebrennie

Left over from debugging?

brenniebrennie

Undo this change.

brenniebrennie

Undo this change.

brenniebrennie

I don't feel like these are states for a review request. Wouldn't a review request be unreviewed and incoming for …

brenniebrennie

Undo this change.

brenniebrennie

Undo this change.

brenniebrennie

Blank line between two blocks.

brenniebrennie

No blank line here.

brenniebrennie

Can you format this like so: elif (self.last_review_request_activity_timetamp is not None and old_reviews is None): site_profile.decrement_unreviewed_outgoing_request_count()

brenniebrennie

Undo this change.

brenniebrennie

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

reviewbotreviewbot

I don't think this filtering is going to work the way you want it to work. We really can't add …

brenniebrennie

See my above comment about why this isn't going to work.

brenniebrennie

Undo this change.

brenniebrennie

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 5 E301 expected 1 blank line, found 0

reviewbotreviewbot

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

reviewbotreviewbot

Col: 5 E301 expected 1 blank line, found 0

reviewbotreviewbot

Col: 33 E231 missing whitespace after ','

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Single quotes for these.

brenniebrennie

These really aren't needed. We only use things like this for enumerated char fields in the database.

brenniebrennie

This should use a named constant.

brenniebrennie

This should use a named constant.

brenniebrennie

Any particular reason not to import this at the top of the file? If there is a good reason, this …

brenniebrennie

Undo this change.

brenniebrennie

This logic can be simplified quite a bit. We are doing query = query & other_query in each case, which …

brenniebrennie

This should do Q(status=status).

brenniebrennie

We should be using named constants for these.

brenniebrennie

This should do Q(status=status). Also can we format this as: query = (query & Q(status=status) & Q(last_review_activity_timestamp=None))

brenniebrennie

Can we call this last_review or last_review_timestamp ?

brenniebrennie

This method needs a docstring that documents what update_counts and old_reviewed do. Also, old_reviewed needs a better name. Perhaps something …

brenniebrennie

ocstrings should be of the following format: """Single line summary. Multi-line description. """

brenniebrennie

Blank line between statement and block.

brenniebrennie

Can we format this as: elif (self.last_review_activity_timestamp is not None and old_reviews is not None):

brenniebrennie

This method needs a docstring. It should also probably be pseudo-private (i.e., it should have a leading underscore).

brenniebrennie

Can we do this as: LocalSiteProfile.objects.filter(local_site=self.local_site, Q(user__review_groups__in=groups) | Q(user__in=people)) Also, can we pull out this query set as a variable …

brenniebrennie

Docstrings should be wrapped at 80. The summary should be short and sweet. How about something like: """Decrease the count …

brenniebrennie

Since we re-use this all over the place, maybe this should be refactored into a nice little helper method? Otherwise …

brenniebrennie

See previous comment.

brenniebrennie

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 19 W503 line break before binary operator

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 19 W503 line break before binary operator

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

This docstring doesn't actually explain what the function does.

brenniebrennie

Needs a docstring.

brenniebrennie

Blank line between these.

brenniebrennie

Need a docstring

brenniebrennie

Leftover from debugging?

brenniebrennie

Blank line between a block and a statement.

brenniebrennie

redefinition of unused 'LocalSiteProfile' from line 12

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/grids.py
        reviewboard/accounts/admin.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/grids.py
        reviewboard/accounts/admin.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
  2. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  3. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  4. reviewboard/datagrids/builtin_items.py (Diff revision 1)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  5. reviewboard/datagrids/builtin_items.py (Diff revision 1)
     
     
    Col: 71
     W291 trailing whitespace
    
  6. reviewboard/datagrids/builtin_items.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  7. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
  2. reviewboard/datagrids/builtin_items.py (Diff revision 2)
     
     
    Col: 71
     W291 trailing whitespace
    
  3. reviewboard/datagrids/builtin_items.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  4. reviewboard/datagrids/grids.py (Diff revision 2)
     
     
    Col: 39
     E231 missing whitespace after ','
    
  5. reviewboard/datagrids/grids.py (Diff revision 2)
     
     
    Col: 39
     E231 missing whitespace after ','
    
  6. reviewboard/reviews/managers.py (Diff revision 2)
     
     
    Col: 5
     E265 block comment should start with '# '
    
  7. reviewboard/reviews/managers.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (89 > 79 characters)
    
  8. reviewboard/reviews/managers.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (88 > 79 characters)
    
  9. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
  2. reviewboard/datagrids/builtin_items.py (Diff revision 3)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  3. reviewboard/datagrids/grids.py (Diff revision 3)
     
     
    Col: 39
     E231 missing whitespace after ','
    
  4. reviewboard/reviews/managers.py (Diff revision 3)
     
     
    Col: 80
     E501 line too long (88 > 79 characters)
    
  5. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 4)
     
     
    Col: 39
     E231 missing whitespace after ','
    
  3. reviewboard/reviews/managers.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (88 > 79 characters)
    
  4. Col: 52
     E711 comparison to None should be 'if cond is None:'
    
  5. Col: 56
     E711 comparison to None should be 'if cond is None:'
    
  6. Col: 56
     E711 comparison to None should be 'if cond is not None:'
    
  7. Col: 80
     E501 line too long (87 > 79 characters)
    
  8. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
  2. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxListView.js
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/accounts/models.py (Diff revision 6)
     
     

    Use single quotes for strings.

  3. reviewboard/datagrids/builtin_items.py (Diff revision 6)
     
     

    three links?

    Also this should be formatted as "All", "Open", and "Unreviewed"

    1. Yes, the unreviewed link is what I am working on

  4. reviewboard/datagrids/builtin_items.py (Diff revision 6)
     
     
     

    Blank line between these.

  5. reviewboard/datagrids/builtin_items.py (Diff revision 6)
     
     
     

    Blank line between these.

  6. reviewboard/datagrids/builtin_items.py (Diff revision 6)
     
     
     

    Blank line between these.

  7. reviewboard/reviews/managers.py (Diff revision 6)
     
     

    Undo this change.

  8. reviewboard/reviews/managers.py (Diff revision 6)
     
     
     
     
     

    Undo this change.

  9. reviewboard/reviews/managers.py (Diff revision 6)
     
     
     
     
     
     

    This doesn't handle the case where status is S or D so this may cause a regression.

    1. But we don't need a section to display the discarded or submitted review requests, do we?

    2. I feel like using an "psuedo" status is a bad idea here. I think it would be better if we have the _query parameter two extra boolean arguments: incoming_unreviewed and outgoing_unreviewed (or similar) and then use those instead of changing the meaning of status.

    3. Yeah, pseudo states are going to get us into some trouble, I fear. That's a pretty core thing. Introducing new possible values for status opens up significant API compatibility and security concerns we'd need to thoroughly test. Plus, there are lots of situations where we do a query for review requests that goes beyond what's default in public(), without having to modify public() itself.

      The Dashboard is the place that needs to deal with these sort of queries, so I'd suggest just having the filter in there. You should be able to do a standard ReviewRequest.objects.public(...).filter(last_review_activity_timestamp=None) without any problems.

  10. reviewboard/reviews/models/review.py (Diff revision 6)
     
     
     

    Undo this change.

  11. reviewboard/reviews/models/review.py (Diff revision 6)
     
     

    Left over from debugging?

  12. reviewboard/reviews/models/review.py (Diff revision 6)
     
     

    Undo this change.

  13. reviewboard/reviews/models/review.py (Diff revision 6)
     
     

    Undo this change.

  14. I don't feel like these are states for a review request. Wouldn't a review request be unreviewed and incoming for a reviewer but unreviewed and outgoing for its submitter?

    1. yes, these statuses are not neccessary, and I will delete these.

  15. Undo this change.

  16. Undo this change.

  17. Blank line between two blocks.

  18. No blank line here.

  19. reviewboard/reviews/models/review_request.py (Diff revision 6)
     
     
     
     
     
     

    Can you format this like so:

    elif (self.last_review_request_activity_timetamp is not None and
           old_reviews is None):
        site_profile.decrement_unreviewed_outgoing_request_count()
    
  20. Undo this change.

  21. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. Col: 29
     E127 continuation line over-indented for visual indent
    
  3. 
      
SE
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/accounts/models.py (Diff revision 8)
     
     
     
     
     
     
     
     
     
     
     
     
     

    I don't think this filtering is going to work the way you want it to work.

    We really can't add the I/O states to a review request because they are per-user things and the review request status is not a per-user thing. We can't filter on status='O' or status='I' here.

    You're probably going to want to use a regular queryset object here, like so:

    initializer=lambda p: ReviewRequest.objects.filter(submitter=p.user, status='P', last_review_activity_timestamp=None).count()

    or something similar. You may want to refactor this into a method on the ReviewRequestManager class.

    1. Yes, I know this place is not going to work, and it is just some of my attemps and failed, but I forgot to remove the codes, really sorry about this and I think I will just delete these codes and try some another methods then.

  3. reviewboard/datagrids/grids.py (Diff revision 8)
     
     
     
     
     
     
     
     
     

    See my above comment about why this isn't going to work.

    1. the codes here I think is going to work because I add the filter to the function from_user and It really filter the unreviewed review requests

  4. Undo this change.

  5. 
      
SE
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/reviews/models/review.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/reviews/models/review.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
    
    
  2. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. reviewboard/accounts/models.py (Diff revision 10)
     
     
    Col: 80
     E501 line too long (87 > 79 characters)
    
  3. reviewboard/accounts/models.py (Diff revision 10)
     
     
    Col: 80
     E501 line too long (87 > 79 characters)
    
  4. reviewboard/reviews/managers.py (Diff revision 10)
     
     
    Col: 5
     E301 expected 1 blank line, found 0
    
  5. reviewboard/reviews/managers.py (Diff revision 10)
     
     
    Col: 80
     E501 line too long (86 > 79 characters)
    
  6. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. reviewboard/reviews/managers.py (Diff revision 11)
     
     
    Col: 5
     E301 expected 1 blank line, found 0
    
  3. reviewboard/reviews/managers.py (Diff revision 11)
     
     
    Col: 33
     E231 missing whitespace after ','
    
  4. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. reviewboard/reviews/managers.py (Diff revision 12)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  3. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. reviewboard/reviews/managers.py (Diff revision 13)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  3. 
      
SE
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/accounts/models.py (Diff revision 14)
     
     

    This should use a named constant.

  3. reviewboard/accounts/models.py (Diff revision 14)
     
     

    This should use a named constant.

  4. reviewboard/reviews/managers.py (Diff revision 14)
     
     

    This should do Q(status=status).

  5. reviewboard/reviews/managers.py (Diff revision 14)
     
     

    We should be using named constants for these.

  6. reviewboard/reviews/managers.py (Diff revision 14)
     
     
     

    This should do Q(status=status).

    Also can we format this as:

    query = (query &
             Q(status=status) &
             Q(last_review_activity_timestamp=None))
    
  7. reviewboard/reviews/models/review.py (Diff revision 14)
     
     

    Can we call this last_review or last_review_timestamp ?

  8. This method needs a docstring that documents what update_counts and old_reviewed do.

    Also, old_reviewed needs a better name. Perhaps something like last_review_timestamp ?

  9. reviewboard/reviews/models/review_request.py (Diff revision 14)
     
     
     

    Blank line between statement and block.

  10. reviewboard/reviews/models/review_request.py (Diff revision 14)
     
     
     

    Can we format this as:

    elif (self.last_review_activity_timestamp is not None and
          old_reviews is not None):
    
  11. This method needs a docstring. It should also probably be pseudo-private (i.e., it should have a leading underscore).

  12. reviewboard/reviews/models/review_request.py (Diff revision 14)
     
     
     
     
     

    Can we do this as:

    LocalSiteProfile.objects.filter(local_site=self.local_site,
                                    Q(user__review_groups__in=groups) | Q(user__in=people))
    

    Also, can we pull out this query set as a variable and pass that along to the decrement function?

  13. reviewboard/reviews/models/review_request.py (Diff revision 14)
     
     
     

    Since we re-use this all over the place, maybe this should be refactored into a nice little helper method?

    Otherwise see other comment about formatting.

  14. reviewboard/reviews/models/review_request.py (Diff revision 14)
     
     
     
     
     

    See previous comment.

  15. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 15)
     
     
    Col: 80
     E501 line too long (111 > 79 characters)
    
  3. reviewboard/datagrids/grids.py (Diff revision 15)
     
     
    Col: 80
     E501 line too long (111 > 79 characters)
    
  4. reviewboard/reviews/managers.py (Diff revision 15)
     
     
    Col: 19
     W503 line break before binary operator
    
  5. reviewboard/reviews/managers.py (Diff revision 15)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  6. reviewboard/reviews/models/review.py (Diff revision 15)
     
     
    Col: 80
     E501 line too long (87 > 79 characters)
    
  7. Col: 19
     W503 line break before binary operator
    
  8. Col: 80
     E501 line too long (112 > 79 characters)
    
  9. Col: 80
     E501 line too long (92 > 79 characters)
    
  10. Col: 80
     E501 line too long (92 > 79 characters)
    
  11. Col: 80
     E501 line too long (92 > 79 characters)
    
  12. Col: 80
     E501 line too long (92 > 79 characters)
    
  13. Col: 80
     E501 line too long (92 > 79 characters)
    
  14. 
      
SE
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. reviewboard/reviews/managers.py (Diff revision 16)
     
     
    Col: 80
     E501 line too long (85 > 79 characters)
    
  3. reviewboard/reviews/managers.py (Diff revision 16)
     
     
    Col: 27
     E127 continuation line over-indented for visual indent
    
  4. Col: 27
     E127 continuation line over-indented for visual indent
    
  5. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/accounts/models.py (Diff revisions 14 - 17)
     
     
     

    Single quotes for these.

  3. reviewboard/accounts/models.py (Diff revisions 14 - 17)
     
     
     
     
     

    These really aren't needed. We only use things like this for enumerated char fields in the database.

  4. reviewboard/datagrids/grids.py (Diff revisions 14 - 17)
     
     

    Any particular reason not to import this at the top of the file?

    If there is a good reason, this should be at the top of the method.

  5. reviewboard/reviews/managers.py (Diff revisions 14 - 15)
     
     

    Undo this change.

  6. reviewboard/reviews/managers.py (Diff revisions 14 - 15)
     
     
     
     
     
     
     
     
     

    This logic can be simplified quite a bit. We are doing query = query & other_query in each case, which can replaced with query &= other_query. We also use query = query & Q(status=status) in both cases, so that can be factored out as follows:

        if status:
            query &= Q(status=status)
    
            if (unreviwed_status in (LocalSiteProfile.OUTGOING_UNREVIEWED,
                                     LocalSiteProfile.INCOMING_UNREVIEWED):
                query &= Q(last_review_activity_timestamp=None)
    
  7. reviewboard/reviews/models/review_request.py (Diff revisions 14 - 15)
     
     
     
     
     
     

    ocstrings should be of the following format:

    """Single line summary.

    Multi-line description.
    """

  8. reviewboard/reviews/models/review_request.py (Diff revisions 14 - 15)
     
     

    Docstrings should be wrapped at 80.

    The summary should be short and sweet. How about something like:

            """Decrease the count of unreviewed incoming requests.
    
            The count is only decreased if a review request that was previously
            unreviewed has become reviewed.
            """
    
  9. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 18)
     
     
     redefinition of unused 'LocalSiteProfile' from line 12
    
  3. Col: 80
     E501 line too long (84 > 79 characters)
    
  4. Col: 80
     E501 line too long (84 > 79 characters)
    
  5. Col: 80
     E501 line too long (83 > 79 characters)
    
  6. Col: 80
     E501 line too long (83 > 79 characters)
    
  7. 
      
SE
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. Col: 80
     E501 line too long (83 > 79 characters)
    
  3. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/reviews/models/review_request.py (Diff revisions 17 - 18)
     
     
     
     
     
     

    This docstring doesn't actually explain what the function does.

  3. reviewboard/reviews/models/review_request.py (Diff revisions 17 - 18)
     
     

    Needs a docstring.

  4. reviewboard/reviews/models/review_request.py (Diff revisions 17 - 18)
     
     
     

    Blank line between these.

  5. reviewboard/reviews/models/review_request.py (Diff revisions 17 - 18)
     
     

    Need a docstring

  6. reviewboard/reviews/models/review_request.py (Diff revisions 17 - 18)
     
     

    Leftover from debugging?

  7. reviewboard/reviews/models/review_request.py (Diff revisions 17 - 18)
     
     
     

    Blank line between a block and a statement.

  8. 
      
SE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/views.py
        reviewboard/reviews/models/review_request.py
        reviewboard/datagrids/grids.py
        reviewboard/datagrids/builtin_items.py
        reviewboard/accounts/models.py
        reviewboard/reviews/managers.py
        reviewboard/accounts/admin.py
        reviewboard/reviews/models/review.py
    
    
  2. 
      
SE
Review request changed

Status: Discarded

Loading...