- Download Diff

Summary:

[WIP] Checklist Extension

Review Request #4131 — Created May 9, 2013 and discarded Jan. 9, 2014, 10:59 p.m.

Information
Owner:	phoenixairs
Repository:	rb-extension-pack
Branch:
Bugs:
Depends On:
Reviewers
Groups:	reviewboard
People:	phoenixairs

Description

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.

Testing Done

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.

Issues

Description	From	Last Updated
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 7:27 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:17 a.m.
Col: 35 W291 trailing whitespace	reviewbot	May 15, 2013, 7:17 a.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 7:27 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:18 a.m.
Col: 42 W291 trailing whitespace	reviewbot	May 15, 2013, 7:18 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:18 a.m.
Col: 42 W291 trailing whitespace	reviewbot	May 15, 2013, 7:18 a.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	May 15, 2013, 7:20 a.m.
Col: 101 W292 no newline at end of file	reviewbot	May 15, 2013, 7:20 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:22 a.m.
Col: 17 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:22 a.m.
Col: 19 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:22 a.m.
Col: 1 W293 blank line contains whitespace	reviewbot	May 15, 2013, 7:22 a.m.
Col: 67 W292 no newline at end of file	reviewbot	May 15, 2013, 7:22 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:25 a.m.
Col: 11 W292 no newline at end of file	reviewbot	May 15, 2013, 7:25 a.m.
Col: 2 W292 no newline at end of file	reviewbot	May 15, 2013, 7:28 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:30 a.m.
Col: 5 W191 indentation contains tabs	reviewbot	May 15, 2013, 7:30 a.m.
Col: 5 E101 indentation contains mixed spaces and tabs	reviewbot	May 15, 2013, 7:30 a.m.
Col: 2 W292 no newline at end of file	reviewbot	May 15, 2013, 7:30 a.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 9, 2013, 12:37 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 9, 2013, 12:37 p.m.
Col: 35 W291 trailing whitespace	reviewbot	May 9, 2013, 12:38 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	May 9, 2013, 12:38 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 9, 2013, 12:38 p.m.
Col: 42 W291 trailing whitespace	reviewbot	May 9, 2013, 12:38 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 9, 2013, 12:38 p.m.
Col: 42 W291 trailing whitespace	reviewbot	May 9, 2013, 12:38 p.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	May 9, 2013, 12:38 p.m.
Col: 101 W292 no newline at end of file	reviewbot	May 9, 2013, 12:38 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 9, 2013, 12:38 p.m.
Col: 17 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:36 a.m.
Col: 19 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 W293 blank line contains whitespace	reviewbot	May 9, 2013, 12:38 p.m.
Col: 67 W292 no newline at end of file	reviewbot	May 9, 2013, 12:38 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 9, 2013, 12:38 p.m.
Col: 11 W292 no newline at end of file	reviewbot	May 9, 2013, 12:38 p.m.
Col: 2 W292 no newline at end of file	reviewbot	May 9, 2013, 12:38 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 9, 2013, 12:38 p.m.
Col: 5 W191 indentation contains tabs	reviewbot	May 9, 2013, 12:39 p.m.
Col: 5 E101 indentation contains mixed spaces and tabs	reviewbot	May 9, 2013, 12:39 p.m.
Col: 2 W292 no newline at end of file	reviewbot	May 9, 2013, 12:39 p.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:36 a.m.
Col: 35 W291 trailing whitespace	reviewbot	May 15, 2013, 7:36 a.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:36 a.m.
Col: 42 W291 trailing whitespace	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:36 a.m.
Col: 42 W291 trailing whitespace	reviewbot	May 15, 2013, 7:36 a.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	May 15, 2013, 7:36 a.m.
Col: 101 W292 no newline at end of file	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:36 a.m.
Col: 17 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:36 a.m.
Col: 19 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 W293 blank line contains whitespace	reviewbot	May 15, 2013, 7:36 a.m.
Col: 67 W292 no newline at end of file	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:36 a.m.
Col: 11 W292 no newline at end of file	reviewbot	May 15, 2013, 7:36 a.m.
Col: 2 W292 no newline at end of file	reviewbot	May 15, 2013, 7:36 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	May 15, 2013, 7:36 a.m.
Col: 5 W191 indentation contains tabs	reviewbot	May 15, 2013, 7:36 a.m.
Col: 5 E101 indentation contains mixed spaces and tabs	reviewbot	May 15, 2013, 7:36 a.m.
Col: 13 E126 continuation line over-indented for hanging indent	reviewbot	May 15, 2013, 7:36 a.m.
Col: 9 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 7:36 a.m.
4 spaces here.	mike_conley	May 15, 2013, 7:38 a.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	May 15, 2013, 7:37 a.m.
Break this up at the second comma.	mike_conley	May 15, 2013, 7:38 a.m.
Why the massive indents? 4 spaces should be enough (or maybe we do 2 for CSS? I don't remember...look through …	mike_conley	May 15, 2013, 7:38 a.m.
What's with the hardcoded ID?	mike_conley	May 14, 2013, 8:25 p.m.
Lowercase "f" in fixed.	mike_conley	May 15, 2013, 7:39 a.m.
var before pk	mike_conley	May 15, 2013, 7:39 a.m.
var before items and max. Semicolon after max = 0.	mike_conley	May 15, 2013, 7:40 a.m.
Spaces on either side of the = and <	mike_conley	May 15, 2013, 7:53 a.m.
Spaces on either side of the +	mike_conley	May 15, 2013, 7:53 a.m.
Spaces on either side of =, <	mike_conley	May 15, 2013, 7:53 a.m.
var items	mike_conley	May 15, 2013, 8:08 a.m.
Space on either side of = and <	mike_conley	May 15, 2013, 8:09 a.m.
var item	mike_conley	May 15, 2013, 8:10 a.m.
Not sure if one-liners are desired... might as well put these in curly braces, like this: if (e.keyCode == 13) …	mike_conley	May 15, 2013, 8:10 a.m.
This comment doesn't make a whole lot of sense...	mike_conley	May 15, 2013, 9:10 a.m.
Switch these two for alphabetical order.	mike_conley	May 15, 2013, 9 a.m.
Switch these two lines.	mike_conley	May 15, 2013, 9 a.m.
Put the imports in alphabetical order - so webapi_check_local_site before webapi_check_login_required.	mike_conley	May 15, 2013, 8:59 a.m.
This should be with the rest of the djblets.webapi imports.	mike_conley	May 15, 2013, 9 a.m.
Col: 17 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:37 a.m.
Col: 19 E251 no spaces around keyword / parameter equals	reviewbot	May 15, 2013, 7:37 a.m.
All of this dead code should be removed.	mike_conley	May 15, 2013, 8:56 a.m.
What's eventually going to go here? What kind of configuration will be necessary?	mike_conley	May 15, 2013, 8:57 a.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 7:37 a.m.
Col: 13 E127 continuation line over-indented for visual indent	reviewbot	May 15, 2013, 7:37 a.m.
Col: 13 E126 continuation line over-indented for hanging indent	reviewbot	May 15, 2013, 7:37 a.m.
Col: 35 W291 trailing whitespace	reviewbot	May 15, 2013, 9:45 a.m.
Col: 48 W291 trailing whitespace	reviewbot	May 15, 2013, 9:45 a.m.
Col: 1 E303 too many blank lines (3)	reviewbot	May 15, 2013, 9:45 a.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 9:45 a.m.
Col: 13 E127 continuation line over-indented for visual indent	reviewbot	May 15, 2013, 9:45 a.m.
Col: 13 E126 continuation line over-indented for hanging indent	reviewbot	May 15, 2013, 9:45 a.m.
Just curious - why are we making the client responsible for setting the IDs? That sounds like something the database …	mike_conley	May 16, 2013, 5:25 p.m.
I think we try to avoid one-liners, so like: if (e.keyCode != 13 \|\| !this.input.val()) { return; }	mike_conley	May 15, 2013, 8:37 p.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 8:43 p.m.
Col: 7 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 8:43 p.m.
Col: 13 E126 continuation line over-indented for hanging indent	reviewbot	May 15, 2013, 8:43 p.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 8:45 p.m.
Col: 7 E128 continuation line under-indented for visual indent	reviewbot	May 15, 2013, 8:45 p.m.
Col: 13 E126 continuation line over-indented for hanging indent	reviewbot	May 15, 2013, 8:45 p.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 16, 2013, 5:26 p.m.
Col: 5 E128 continuation line under-indented for visual indent	reviewbot	May 16, 2013, 5:26 p.m.
Col: 13 E126 continuation line over-indented for hanging indent	reviewbot	May 16, 2013, 5:26 p.m.

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/templatetags/checklisttags.pyc
    rbchecklist/rbchecklist/htdocs/js/checklists.js
    rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js
    rbchecklist/rbchecklist/templatetags/__init__.pyc
    rbchecklist/rbchecklist/templates/rbchecklist/checklist.html

rbchecklist/rbchecklist/admin_urls.py (Diff revision 1)

Show all issues

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

rbchecklist/rbchecklist/extension.py (Diff revision 1)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/extension.py (Diff revision 1)
Show all issues
```
Col: 35
 W291 trailing whitespace
```

rbchecklist/rbchecklist/extension.py (Diff revision 1)

Show all issues

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

rbchecklist/rbchecklist/models.py (Diff revision 1)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/models.py (Diff revision 1)
Show all issues
```
Col: 42
 W291 trailing whitespace
```
rbchecklist/rbchecklist/models.py (Diff revision 1)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/models.py (Diff revision 1)
Show all issues
```
Col: 42
 W291 trailing whitespace
```
rbchecklist/rbchecklist/models.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (100 > 79 characters)
```
rbchecklist/rbchecklist/models.py (Diff revision 1)
Show all issues
```
Col: 101
 W292 no newline at end of file
```
rbchecklist/rbchecklist/resources.py (Diff revision 1)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/resources.py (Diff revision 1)
Show all issues
```
Col: 17
 E251 no spaces around keyword / parameter equals
```
rbchecklist/rbchecklist/resources.py (Diff revision 1)
Show all issues
```
Col: 19
 E251 no spaces around keyword / parameter equals
```
rbchecklist/rbchecklist/resources.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
rbchecklist/rbchecklist/resources.py (Diff revision 1)
Show all issues
```
Col: 67
 W292 no newline at end of file
```
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 1)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 1)
Show all issues
```
Col: 11
 W292 no newline at end of file
```
rbchecklist/rbchecklist/urls.py (Diff revision 1)
Show all issues
```
Col: 2
 W292 no newline at end of file
```
rbchecklist/rbchecklist/views.py (Diff revision 1)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/views.py (Diff revision 1)
Show all issues
```
Col: 5
 W191 indentation contains tabs
```

rbchecklist/rbchecklist/views.py (Diff revision 1)

Show all issues

Col: 5
 E101 indentation contains mixed spaces and tabs

rbchecklist/setup.py (Diff revision 1)
Show all issues
```
Col: 2
 W292 no newline at end of file
```

Change Summary:

Fixed PEP styling issues

Diff:

Revision 2 (+497)

Show changes

	rbchecklist/setup.py
	rbchecklist/rbchecklist/admin_urls.py
	rbchecklist/rbchecklist/extension.py
	rbchecklist/rbchecklist/models.py
	rbchecklist/rbchecklist/resources.py
	rbchecklist/rbchecklist/urls.py
	rbchecklist/rbchecklist/views.py
	rbchecklist/rbchecklist/htdocs/js/checklists.js
	9 more

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)

Show all issues

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

rbchecklist/rbchecklist/extension.py (Diff revision 2)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/extension.py (Diff revision 2)
Show all issues
```
Col: 35
 W291 trailing whitespace
```

rbchecklist/rbchecklist/extension.py (Diff revision 2)

Show all issues

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

rbchecklist/rbchecklist/models.py (Diff revision 2)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/models.py (Diff revision 2)
Show all issues
```
Col: 42
 W291 trailing whitespace
```
rbchecklist/rbchecklist/models.py (Diff revision 2)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/models.py (Diff revision 2)
Show all issues
```
Col: 42
 W291 trailing whitespace
```
rbchecklist/rbchecklist/models.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (100 > 79 characters)
```
rbchecklist/rbchecklist/models.py (Diff revision 2)
Show all issues
```
Col: 101
 W292 no newline at end of file
```
rbchecklist/rbchecklist/resources.py (Diff revision 2)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/resources.py (Diff revision 2)
Show all issues
```
Col: 17
 E251 no spaces around keyword / parameter equals
```
rbchecklist/rbchecklist/resources.py (Diff revision 2)
Show all issues
```
Col: 19
 E251 no spaces around keyword / parameter equals
```
rbchecklist/rbchecklist/resources.py (Diff revision 2)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
rbchecklist/rbchecklist/resources.py (Diff revision 2)
Show all issues
```
Col: 67
 W292 no newline at end of file
```
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 2)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 2)
Show all issues
```
Col: 11
 W292 no newline at end of file
```
rbchecklist/rbchecklist/urls.py (Diff revision 2)
Show all issues
```
Col: 2
 W292 no newline at end of file
```
rbchecklist/rbchecklist/views.py (Diff revision 2)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/views.py (Diff revision 2)
Show all issues
```
Col: 5
 W191 indentation contains tabs
```

rbchecklist/rbchecklist/views.py (Diff revision 2)

Show all issues

Col: 5
 E101 indentation contains mixed spaces and tabs

rbchecklist/setup.py (Diff revision 2)
Show all issues
```
Col: 2
 W292 no newline at end of file
```

Change Summary:

Actually committed PEP style fixes this time.

Diff:

Revision 3 (+497)

Show changes

	rbchecklist/setup.py
	rbchecklist/rbchecklist/admin_urls.py
	rbchecklist/rbchecklist/extension.py
	rbchecklist/rbchecklist/models.py
	rbchecklist/rbchecklist/resources.py
	rbchecklist/rbchecklist/urls.py
	rbchecklist/rbchecklist/views.py
	rbchecklist/rbchecklist/htdocs/js/checklists.js
	9 more

Change Summary:

Actually committed PEP style fixes this time.

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)

Show all issues

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

rbchecklist/rbchecklist/extension.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/extension.py (Diff revision 3)
Show all issues
```
Col: 35
 W291 trailing whitespace
```

rbchecklist/rbchecklist/extension.py (Diff revision 3)

Show all issues

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

rbchecklist/rbchecklist/models.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/models.py (Diff revision 3)
Show all issues
```
Col: 42
 W291 trailing whitespace
```
rbchecklist/rbchecklist/models.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/models.py (Diff revision 3)
Show all issues
```
Col: 42
 W291 trailing whitespace
```
rbchecklist/rbchecklist/models.py (Diff revision 3)
Show all issues
```
Col: 80
 E501 line too long (100 > 79 characters)
```
rbchecklist/rbchecklist/models.py (Diff revision 3)
Show all issues
```
Col: 101
 W292 no newline at end of file
```
rbchecklist/rbchecklist/resources.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/resources.py (Diff revision 3)
Show all issues
```
Col: 17
 E251 no spaces around keyword / parameter equals
```
rbchecklist/rbchecklist/resources.py (Diff revision 3)
Show all issues
```
Col: 19
 E251 no spaces around keyword / parameter equals
```
rbchecklist/rbchecklist/resources.py (Diff revision 3)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
rbchecklist/rbchecklist/resources.py (Diff revision 3)
Show all issues
```
Col: 67
 W292 no newline at end of file
```
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/templatetags/checklisttags.py (Diff revision 3)
Show all issues
```
Col: 11
 W292 no newline at end of file
```
rbchecklist/rbchecklist/urls.py (Diff revision 3)
Show all issues
```
Col: 2
 W292 no newline at end of file
```
rbchecklist/rbchecklist/views.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
rbchecklist/rbchecklist/views.py (Diff revision 3)
Show all issues
```
Col: 5
 W191 indentation contains tabs
```

rbchecklist/rbchecklist/views.py (Diff revision 3)

Show all issues

Col: 5
 E101 indentation contains mixed spaces and tabs

rbchecklist/setup.py (Diff revision 3)

Show all issues

Col: 13
 E126 continuation line over-indented for hanging indent

Summary:

-	Checklist on own page (First pass)
+	[WIP] This is now used to stage for style checks >.>

Groups:

-	reviewboard

People:

+	phoenixairs

Change Summary:

Aesthetics and code cleanup

Diff:

Revision 4 (+498)

Show changes

	rbchecklist/setup.py
	rbchecklist/rbchecklist/admin_urls.py
	rbchecklist/rbchecklist/extension.py
	rbchecklist/rbchecklist/models.py
	rbchecklist/rbchecklist/resources.py
	rbchecklist/rbchecklist/urls.py
	rbchecklist/rbchecklist/views.py
	rbchecklist/rbchecklist/htdocs/css/checklists.css
	7 more

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)

Show all issues

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

rbchecklist/rbchecklist/extension.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
rbchecklist/rbchecklist/resources.py (Diff revision 4)
Show all issues
```
Col: 17
 E251 no spaces around keyword / parameter equals
```
rbchecklist/rbchecklist/resources.py (Diff revision 4)
Show all issues
```
Col: 19
 E251 no spaces around keyword / parameter equals
```

rbchecklist/rbchecklist/urls.py (Diff revision 4)

Show all issues

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

rbchecklist/rbchecklist/urls.py (Diff revision 4)

Show all issues

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

rbchecklist/setup.py (Diff revision 4)

Show all issues

Col: 13
 E126 continuation line over-indented for hanging indent

Groups:

+	reviewboard

Fix it!

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

phoenixairs May 15, 2013, 9:36 a.m.

Filled in the testing section.

The checklist is visible as long as the global variable gReviewRequest is active, so it's available while viewing screenshots, diffs, and binary files.

rbchecklist/rbchecklist/admin_urls.py (Diff revision 4)
Show all issues
```
4 spaces here.
```
rbchecklist/rbchecklist/extension.py (Diff revision 4)
Show all issues
```
Break this up at the second comma.
```

rbchecklist/rbchecklist/htdocs/css/checklists.css (Diff revision 4)

Show all issues

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/checklists.js (Diff revision 4)
Show all issues
```
What's with the hardcoded ID?
```
rbchecklist/rbchecklist/htdocs/js/checklists.js (Diff revision 4)
Show all issues
```
Lowercase "f" in fixed.
```
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4)
Show all issues
```
var before pk
```
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4)
Show all issues
```
var before items and max. Semicolon after max = 0.
```
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4)
Show all issues
```
Spaces on either side of the = and <
```
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4)
Show all issues
```
Spaces on either side of the +
```
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4)
Show all issues
```
Spaces on either side of =, <
```
rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 4)
Show all issues
```
var items
```
rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 4)
```
var items
```
rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 4)
Show all issues
```
Space on either side of = and <
```
rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 4)
Show all issues
```
var item
```
rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 4)
```
Why not || these?
```
1. PH
  
  phoenixairs May 15, 2013, 9:36 a.m.
  ||ed

rbchecklist/rbchecklist/htdocs/js/views/checklistItemView.js (Diff revision 4)

Show all issues

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)
Show all issues
```
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)
Show all issues
```
Switch these two for alphabetical order.
```
rbchecklist/rbchecklist/resources.py (Diff revision 4)
Show all issues
```
Switch these two lines.
```

rbchecklist/rbchecklist/resources.py (Diff revision 4)

Show all issues

Put the imports in alphabetical order - so webapi_check_local_site before webapi_check_login_required.

rbchecklist/rbchecklist/resources.py (Diff revision 4)

Show all issues

This should be with the rest of the djblets.webapi imports.

rbchecklist/rbchecklist/resources.py (Diff revision 4)

Show all issues

All of this dead code should be removed.

rbchecklist/rbchecklist/templates/rbchecklist/configure.html (Diff revision 4)

Show all issues

What's eventually going to go here? What kind of configuration will be necessary?

phoenixairs May 15, 2013, 9:36 a.m.

Removed this, don't actually have anything in mind.

Summary:

-	[WIP] This is now used to stage for style checks >.>
+	[WIP] Checklist Extension

Change Summary:

Updated description / testing

Description:

		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.

Testing Done:

	+	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.

Change Summary:

Changes in response to m_conley's review

Diff:

Revision 5 (+465)

Show changes

	rbchecklist/setup.py
	rbchecklist/rbchecklist/extension.py
	rbchecklist/rbchecklist/models.py
	rbchecklist/rbchecklist/resources.py
	rbchecklist/rbchecklist/urls.py
	rbchecklist/rbchecklist/htdocs/css/checklists.css
	rbchecklist/rbchecklist/htdocs/js/checklists.js
	rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js
	4 more

Fix it!

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/extension.py (Diff revision 5)
Show all issues
```
Col: 35
 W291 trailing whitespace
```
rbchecklist/rbchecklist/extension.py (Diff revision 5)
Show all issues
```
Col: 48
 W291 trailing whitespace
```
rbchecklist/rbchecklist/resources.py (Diff revision 5)
Show all issues
```
Col: 1
 E303 too many blank lines (3)
```

rbchecklist/rbchecklist/urls.py (Diff revision 5)

Show all issues

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

rbchecklist/rbchecklist/urls.py (Diff revision 5)

Show all issues

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

rbchecklist/setup.py (Diff revision 5)

Show all issues

Col: 13
 E126 continuation line over-indented for hanging indent

Change Summary:

PEP style fixes

Diff:

Revision 6 (+462)

Show changes

	rbchecklist/setup.py
	rbchecklist/rbchecklist/extension.py
	rbchecklist/rbchecklist/models.py
	rbchecklist/rbchecklist/resources.py
	rbchecklist/rbchecklist/urls.py
	rbchecklist/rbchecklist/htdocs/css/checklists.css
	rbchecklist/rbchecklist/htdocs/js/checklists.js
	rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js
	4 more

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)

Show all issues

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

rbchecklist/rbchecklist/urls.py (Diff revision 6)

Show all issues

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

rbchecklist/setup.py (Diff revision 6)

Show all issues

Col: 13
 E126 continuation line over-indented for hanging indent

Fix it!

Hey Felix,

Looking really good - just two more points during this last pass.

-Mike

rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js (Diff revision 6)

Show all issues

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?

phoenixairs May 15, 2013, 8:45 p.m.

This is the item's index within the checklist. All the items in a checklist are bundled into a JSON string, and the JSON string is what is sent to the server, so the database won't be able to see the item or set an ID.

The column "template_json" in the database is storing something like '[{id:1, text:"This is a checklist item"}, {id:2, text:"This is another checklist item"}]'

mike_conley May 16, 2013, 6:56 a.m.

Ah, ok - feel free to drop this one.

Another question though - it seems a bit strange to recalculate that value each time an item is added. Can't we calculate it once and cache it?

-Mike

phoenixairs May 16, 2013, 6:37 p.m.

You're right. I rewrote ChecklistInstance model to store the maxIndex and use it when adding items.

rbchecklist/rbchecklist/htdocs/js/views/checklistInstanceView.js (Diff revision 6)

Show all issues

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)

Show changes

	rbchecklist/setup.py
	rbchecklist/rbchecklist/extension.py
	rbchecklist/rbchecklist/models.py
	rbchecklist/rbchecklist/resources.py
	rbchecklist/rbchecklist/urls.py
	rbchecklist/rbchecklist/htdocs/css/checklists.css
	rbchecklist/rbchecklist/htdocs/js/checklists.js
	rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js
	4 more

Fix it!

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)

Show all issues

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

rbchecklist/rbchecklist/urls.py (Diff revision 7)

Show all issues

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

rbchecklist/setup.py (Diff revision 7)

Show all issues

Col: 13
 E126 continuation line over-indented for hanging indent

Change Summary:

Rewrote checklistInstanceModel to cache the maxIndex

Diff:

Revision 8 (+473)

Show changes

	rbchecklist/setup.py
	rbchecklist/rbchecklist/extension.py
	rbchecklist/rbchecklist/models.py
	rbchecklist/rbchecklist/resources.py
	rbchecklist/rbchecklist/urls.py
	rbchecklist/rbchecklist/htdocs/css/checklists.css
	rbchecklist/rbchecklist/htdocs/js/checklists.js
	rbchecklist/rbchecklist/htdocs/js/models/checklistInstanceModel.js
	4 more

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)

Show all issues

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

rbchecklist/rbchecklist/urls.py (Diff revision 8)

Show all issues

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

rbchecklist/setup.py (Diff revision 8)

Show all issues

Col: 13
 E126 continuation line over-indented for hanging indent

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

[WIP] Checklist Extension

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Summary:

Groups:

People:

Change Summary:

Diff:

Groups:

Summary:

Change Summary:

Description:

Testing Done:

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Status: Discarded