-
-
rbchecklist/rbchecklist/admin_urls.py (Diff revision 1) Col: 5 E128 continuation line under-indented for visual indent
-
-
-
rbchecklist/rbchecklist/extension.py (Diff revision 1) Col: 13 E128 continuation line under-indented for visual indent
-
-
-
-
-
rbchecklist/rbchecklist/models.py (Diff revision 1) Col: 80 E501 line too long (100 > 79 characters)
-
-
-
rbchecklist/rbchecklist/resources.py (Diff revision 1) Col: 17 E251 no spaces around keyword / parameter equals
-
rbchecklist/rbchecklist/resources.py (Diff revision 1) Col: 19 E251 no spaces around keyword / parameter equals
-
-
-
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 1) Col: 1 E302 expected 2 blank lines, found 1
-
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 1) Col: 11 W292 no newline at end of file
-
-
-
-
rbchecklist/rbchecklist/views.py (Diff revision 1) Col: 5 E101 indentation contains mixed spaces and tabs
-
[WIP] Checklist Extension
Review Request #4131 — Created May 9, 2013 and discarded
Information | |
---|---|
phoenixairs | |
rb-extension-pack | |
Reviewers | |
reviewboard | |
phoenixairs |
First iteration of Checklist extension Milestone: Checklist on own page, can add/delete items and check items off Changes persist in database Includes: Extension configuration files Django models Backbone models/views Remaining work: Currently hardcoded to request ChecklistInstance with id=1. Need to figure out how to create or retrieve a ChecklistInstance associated with the current Review(Draft)? in progress.
Currently need to hardcode first Checklist / ChecklistInstance in, with the JSON = [] (empty list) Went to home page, extension page: checklist does not appear Go to a review request: checklist appears. Should be visible on all review views. Add items by filling in textfield and hitting enter Update items by double-clicking their text, editing the text, and hitting enter Delete item same way as updating, but set text to be empty Check and uncheck items Refresh or leave the review, then come back and confirm changes persist. Drag the dialog around. Scroll the page. Dialog should stay in place. Attached a screenshot and opened it. Checklist is still visible.
Description | From | Last Updated |
---|---|---|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 35 W291 trailing whitespace |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 42 W291 trailing whitespace |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 42 W291 trailing whitespace |
![]() |
|
Col: 80 E501 line too long (100 > 79 characters) |
![]() |
|
Col: 101 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 17 E251 no spaces around keyword / parameter equals |
![]() |
|
Col: 19 E251 no spaces around keyword / parameter equals |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 67 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 11 W292 no newline at end of file |
![]() |
|
Col: 2 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 5 W191 indentation contains tabs |
![]() |
|
Col: 5 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 2 W292 no newline at end of file |
![]() |
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 35 W291 trailing whitespace |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 42 W291 trailing whitespace |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 42 W291 trailing whitespace |
![]() |
|
Col: 80 E501 line too long (100 > 79 characters) |
![]() |
|
Col: 101 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 17 E251 no spaces around keyword / parameter equals |
![]() |
|
Col: 19 E251 no spaces around keyword / parameter equals |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 67 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 11 W292 no newline at end of file |
![]() |
|
Col: 2 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 5 W191 indentation contains tabs |
![]() |
|
Col: 5 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 2 W292 no newline at end of file |
![]() |
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 35 W291 trailing whitespace |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 42 W291 trailing whitespace |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 42 W291 trailing whitespace |
![]() |
|
Col: 80 E501 line too long (100 > 79 characters) |
![]() |
|
Col: 101 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 17 E251 no spaces around keyword / parameter equals |
![]() |
|
Col: 19 E251 no spaces around keyword / parameter equals |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 67 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 11 W292 no newline at end of file |
![]() |
|
Col: 2 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 5 W191 indentation contains tabs |
![]() |
|
Col: 5 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 13 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 9 E128 continuation line under-indented for visual indent |
![]() |
|
4 spaces here. |
|
|
Col: 80 E501 line too long (85 > 79 characters) |
![]() |
|
Break this up at the second comma. |
|
|
Why the massive indents? 4 spaces should be enough (or maybe we do 2 for CSS? I don't remember...look through … |
|
|
What's with the hardcoded ID? |
|
|
Lowercase "f" in fixed. |
|
|
var before pk |
|
|
var before items and max. Semicolon after max = 0. |
|
|
Spaces on either side of the = and < |
|
|
Spaces on either side of the + |
|
|
Spaces on either side of =, < |
|
|
var items |
|
|
Space on either side of = and < |
|
|
var item |
|
|
Not sure if one-liners are desired... might as well put these in curly braces, like this: if (e.keyCode == 13) … |
|
|
This comment doesn't make a whole lot of sense... |
|
|
Switch these two for alphabetical order. |
|
|
Switch these two lines. |
|
|
Put the imports in alphabetical order - so webapi_check_local_site before webapi_check_login_required. |
|
|
This should be with the rest of the djblets.webapi imports. |
|
|
Col: 17 E251 no spaces around keyword / parameter equals |
![]() |
|
Col: 19 E251 no spaces around keyword / parameter equals |
![]() |
|
All of this dead code should be removed. |
|
|
What's eventually going to go here? What kind of configuration will be necessary? |
|
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 13 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 35 W291 trailing whitespace |
![]() |
|
Col: 48 W291 trailing whitespace |
![]() |
|
Col: 1 E303 too many blank lines (3) |
![]() |
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 13 E126 continuation line over-indented for hanging indent |
![]() |
|
Just curious - why are we making the client responsible for setting the IDs? That sounds like something the database … |
|
|
I think we try to avoid one-liners, so like: if (e.keyCode != 13 || !this.input.val()) { return; } |
|
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 7 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 7 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 5 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E126 continuation line over-indented for hanging indent |
![]() |


-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbchecklist/rbchecklist/admin_urls.py rbchecklist/rbchecklist/models.py rbchecklist/rbchecklist/extension.py rbchecklist/setup.py rbchecklist/rbchecklist/resources.py rbchecklist/rbchecklist/templatetags/checklisttags.py rbchecklist/rbchecklist/urls.py rbchecklist/rbchecklist/views.py Ignored Files: rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js rbchecklist/rbchecklist/htdocs/js/models/checklistItemModel.js rbchecklist/rbchecklist/templates/rbchecklist/configure.html rbchecklist/rbchecklist/templates/rbchecklist/scripts.html rbchecklist/rbchecklist/templates/rbchecklist/base.html rbchecklist/rbchecklist/htdocs/js/checklists.js rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js rbchecklist/rbchecklist/templates/rbchecklist/checklist.html
-
rbchecklist/rbchecklist/admin_urls.py (Diff revision 2) Col: 5 E128 continuation line under-indented for visual indent
-
-
-
rbchecklist/rbchecklist/extension.py (Diff revision 2) Col: 13 E128 continuation line under-indented for visual indent
-
-
-
-
-
rbchecklist/rbchecklist/models.py (Diff revision 2) Col: 80 E501 line too long (100 > 79 characters)
-
-
-
rbchecklist/rbchecklist/resources.py (Diff revision 2) Col: 17 E251 no spaces around keyword / parameter equals
-
rbchecklist/rbchecklist/resources.py (Diff revision 2) Col: 19 E251 no spaces around keyword / parameter equals
-
-
-
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 2) Col: 1 E302 expected 2 blank lines, found 1
-
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 2) Col: 11 W292 no newline at end of file
-
-
-
-
rbchecklist/rbchecklist/views.py (Diff revision 2) Col: 5 E101 indentation contains mixed spaces and tabs
-

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbchecklist/rbchecklist/models.py rbchecklist/rbchecklist/admin_urls.py rbchecklist/rbchecklist/extension.py rbchecklist/setup.py rbchecklist/rbchecklist/resources.py rbchecklist/rbchecklist/templatetags/checklisttags.py rbchecklist/rbchecklist/urls.py rbchecklist/rbchecklist/views.py Ignored Files: rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js rbchecklist/rbchecklist/htdocs/js/models/checklistItemModel.js rbchecklist/rbchecklist/templates/rbchecklist/configure.html rbchecklist/rbchecklist/templates/rbchecklist/scripts.html rbchecklist/rbchecklist/templates/rbchecklist/base.html rbchecklist/rbchecklist/htdocs/js/checklists.js rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js rbchecklist/rbchecklist/templates/rbchecklist/checklist.html
-
rbchecklist/rbchecklist/admin_urls.py (Diff revision 3) Col: 5 E128 continuation line under-indented for visual indent
-
-
-
rbchecklist/rbchecklist/extension.py (Diff revision 3) Col: 13 E128 continuation line under-indented for visual indent
-
-
-
-
-
rbchecklist/rbchecklist/models.py (Diff revision 3) Col: 80 E501 line too long (100 > 79 characters)
-
-
-
rbchecklist/rbchecklist/resources.py (Diff revision 3) Col: 17 E251 no spaces around keyword / parameter equals
-
rbchecklist/rbchecklist/resources.py (Diff revision 3) Col: 19 E251 no spaces around keyword / parameter equals
-
-
-
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 3) Col: 1 E302 expected 2 blank lines, found 1
-
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 3) Col: 11 W292 no newline at end of file
-
-
-
-
rbchecklist/rbchecklist/views.py (Diff revision 3) Col: 5 E101 indentation contains mixed spaces and tabs
-
rbchecklist/setup.py (Diff revision 3) Col: 13 E126 continuation line over-indented for hanging indent
Summary: |
|
||||
---|---|---|---|---|---|
Groups: |
|
||||
People: |
|

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbchecklist/rbchecklist/admin_urls.py rbchecklist/rbchecklist/models.py rbchecklist/rbchecklist/extension.py rbchecklist/setup.py rbchecklist/rbchecklist/resources.py rbchecklist/rbchecklist/templatetags/checklisttags.py rbchecklist/rbchecklist/urls.py rbchecklist/rbchecklist/views.py Ignored Files: rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js rbchecklist/rbchecklist/htdocs/js/models/checklistItemModel.js rbchecklist/rbchecklist/templates/rbchecklist/configure.html rbchecklist/rbchecklist/htdocs/css/checklists.css rbchecklist/rbchecklist/htdocs/js/checklists.js rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js
-
rbchecklist/rbchecklist/admin_urls.py (Diff revision 4) Col: 9 E128 continuation line under-indented for visual indent
-
rbchecklist/rbchecklist/extension.py (Diff revision 4) Col: 80 E501 line too long (85 > 79 characters)
-
rbchecklist/rbchecklist/resources.py (Diff revision 4) Col: 17 E251 no spaces around keyword / parameter equals
-
rbchecklist/rbchecklist/resources.py (Diff revision 4) Col: 19 E251 no spaces around keyword / parameter equals
-
rbchecklist/rbchecklist/urls.py (Diff revision 4) Col: 5 E128 continuation line under-indented for visual indent
-
rbchecklist/rbchecklist/urls.py (Diff revision 4) Col: 13 E127 continuation line over-indented for visual indent
-
rbchecklist/setup.py (Diff revision 4) Col: 13 E126 continuation line over-indented for hanging indent
-
Hey Felix, This looks pretty good. Please address these issues, along with the ones ReviewBot picked up (or drop the ones that don't make sense, along with an explanation on why you dropped it). You also need to fill in the "Testing Done" field of this review request. Tell us what scenarios you've tested. I've also not tried the extension, but I've seen the video...is the checklist available while viewing diffs, screenshots and other binary files? -Mike
-
-
-
rbchecklist/rbchecklist/htdocs/css/checklists.css (Diff revision 4) Why the massive indents? 4 spaces should be enough (or maybe we do 2 for CSS? I don't remember...look through some of our Review Board CSS for the correct format).
-
-
-
-
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4) var before items and max. Semicolon after max = 0.
-
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4) Spaces on either side of the = and <
-
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4) Spaces on either side of the +
-
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4) Spaces on either side of =, <
-
-
-
rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 4) Space on either side of = and <
-
-
rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 4) Why not || these?
-
rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js (Diff revision 4) Not sure if one-liners are desired... might as well put these in curly braces, like this: if (e.keyCode == 13) { this.close(); }
-
rbchecklist/rbchecklist/models.py (Diff revision 4) This comment doesn't make a whole lot of sense...
-
rbchecklist/rbchecklist/models.py (Diff revision 4) These two lines of documentation aren't really clear... should this be: Checklist items for this template are stored as a JSON string in the following format:
-
-
-
rbchecklist/rbchecklist/resources.py (Diff revision 4) Put the imports in alphabetical order - so webapi_check_local_site before webapi_check_login_required.
-
rbchecklist/rbchecklist/resources.py (Diff revision 4) This should be with the rest of the djblets.webapi imports.
-
-
rbchecklist/rbchecklist/templates/rbchecklist/configure.html (Diff revision 4) What's eventually going to go here? What kind of configuration will be necessary?
Change Summary:
Updated description / testing
Description: |
|
||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
Change Summary:
Changes in response to m_conley's review

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbchecklist/rbchecklist/models.py rbchecklist/setup.py rbchecklist/rbchecklist/extension.py rbchecklist/rbchecklist/urls.py rbchecklist/rbchecklist/resources.py Ignored Files: rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js rbchecklist/rbchecklist/htdocs/js/models/checklistItemModel.js rbchecklist/rbchecklist/htdocs/js/checklists.js rbchecklist/rbchecklist/htdocs/css/checklists.css rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js rbchecklist/rbchecklist/templates/rbchecklist/checklist_scripts.html
-
-
-
-
rbchecklist/rbchecklist/urls.py (Diff revision 5) Col: 5 E128 continuation line under-indented for visual indent
-
rbchecklist/rbchecklist/urls.py (Diff revision 5) Col: 13 E127 continuation line over-indented for visual indent
-
rbchecklist/setup.py (Diff revision 5) Col: 13 E126 continuation line over-indented for hanging indent
Change Summary:
PEP style fixes
Diff: |
Revision 6 (+462)
|
---|

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbchecklist/rbchecklist/models.py rbchecklist/setup.py rbchecklist/rbchecklist/extension.py rbchecklist/rbchecklist/urls.py rbchecklist/rbchecklist/resources.py Ignored Files: rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js rbchecklist/rbchecklist/htdocs/js/models/checklistItemModel.js rbchecklist/rbchecklist/htdocs/js/checklists.js rbchecklist/rbchecklist/htdocs/css/checklists.css rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js rbchecklist/rbchecklist/templates/rbchecklist/checklist_scripts.html
-
rbchecklist/rbchecklist/urls.py (Diff revision 6) Col: 5 E128 continuation line under-indented for visual indent
-
rbchecklist/rbchecklist/urls.py (Diff revision 6) Col: 7 E128 continuation line under-indented for visual indent
-
rbchecklist/setup.py (Diff revision 6) Col: 13 E126 continuation line over-indented for hanging indent
-
Hey Felix, Looking really good - just two more points during this last pass. -Mike
-
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 6) Just curious - why are we making the client responsible for setting the IDs? That sounds like something the database could / should probably take care of automatically, no?
-
rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 6) I think we try to avoid one-liners, so like: if (e.keyCode != 13 || !this.input.val()) { return; }
Change Summary:
Fixed the one-liner that m_conley pointed out.
Diff: |
Revision 7 (+462)
|
---|

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbchecklist/rbchecklist/models.py rbchecklist/setup.py rbchecklist/rbchecklist/extension.py rbchecklist/rbchecklist/urls.py rbchecklist/rbchecklist/resources.py Ignored Files: rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js rbchecklist/rbchecklist/htdocs/js/models/checklistItemModel.js rbchecklist/rbchecklist/htdocs/js/checklists.js rbchecklist/rbchecklist/htdocs/css/checklists.css rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js rbchecklist/rbchecklist/templates/rbchecklist/checklist_scripts.html
-
rbchecklist/rbchecklist/urls.py (Diff revision 7) Col: 5 E128 continuation line under-indented for visual indent
-
rbchecklist/rbchecklist/urls.py (Diff revision 7) Col: 7 E128 continuation line under-indented for visual indent
-
rbchecklist/setup.py (Diff revision 7) Col: 13 E126 continuation line over-indented for hanging indent
Change Summary:
Rewrote checklistInstanceModel to cache the maxIndex
Diff: |
Revision 8 (+473)
|
---|

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbchecklist/rbchecklist/models.py rbchecklist/setup.py rbchecklist/rbchecklist/extension.py rbchecklist/rbchecklist/urls.py rbchecklist/rbchecklist/resources.py Ignored Files: rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js rbchecklist/rbchecklist/htdocs/js/models/checklistItemModel.js rbchecklist/rbchecklist/htdocs/js/checklists.js rbchecklist/rbchecklist/htdocs/css/checklists.css rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js rbchecklist/rbchecklist/templates/rbchecklist/checklist_scripts.html
-
rbchecklist/rbchecklist/urls.py (Diff revision 8) Col: 5 E128 continuation line under-indented for visual indent
-
rbchecklist/rbchecklist/urls.py (Diff revision 8) Col: 5 E128 continuation line under-indented for visual indent
-
rbchecklist/setup.py (Diff revision 8) Col: 13 E126 continuation line over-indented for hanging indent