-
-
-
reviewboard/site/validation.py (Diff revision 1) Input on the message here would be appreciated too.
Add additional validation to bug tracker URLs
Review Request #3378 — Created Sept. 29, 2012 and submitted
| Information | |
|---|---|
| Karl-L | |
| Review Board | |
| 1996 | |
| Reviewers | |
| reviewboard | |
When specifying a custom bug tracker URL, the URL is passed through additional validation to ensure that there is a '%s' string formatting element.
Tested on local development server against custom urls and a new GitHub repository's issue tracker. Added unit test for validator.
| Description | From | Last Updated |
|---|---|---|
|
We tend to do a lot of stuff alphabetically, so these should be imported in this order: validate_bug_tracker, validate_review_groups, validate_users. |
 mike_conley
|
|
|
Would love some input on the message. |
KA Karl-L | |
|
As before, we tend to do things alphabetically - so django.forms stuff should be imported before django.http stuff. |
mike_conley
|
|
|
There should be a docstring here say what this test is doing. |
mike_conley
|
|
|
Input on the message here would be appreciated too. |
KA Karl-L | |
|
reviewboard.site isn't the right place for validate_bug_tracker. Can you move it to reviewboard/admin/validation.py (which doesn't exist yet) and move the … |
 david
|
KA
-
Hey Karl, This looks really good! Just a few suggestions - see below. -Mike
-
reviewboard/scmtools/forms.py (Diff revision 1) We tend to do a lot of stuff alphabetically, so these should be imported in this order: validate_bug_tracker, validate_review_groups, validate_users.
-
reviewboard/scmtools/forms.py (Diff revision 1) How about "The path should resemble: http://www.example.com/issues?id=%s where %s will be the bug number.
-
reviewboard/site/tests.py (Diff revision 1) As before, we tend to do things alphabetically - so django.forms stuff should be imported before django.http stuff.
-
reviewboard/site/tests.py (Diff revision 1) There should be a docstring here say what this test is doing.
-
KA
Change Summary:
Fixed a few import ordering issues (made alphabetical), updated some copy and fixed test so that it can fail (instead of just error).
Diff: |
Revision 2 (+54 -2) |
|---|
-
-
reviewboard/scmtools/forms.py (Diff revision 2) reviewboard.site isn't the right place for validate_bug_tracker. Can you move it to reviewboard/admin/validation.py (which doesn't exist yet) and move the test into reviewboard/admin/tests.py ?
KA
Change Summary:
Move validator and test to correct app -- in reviewboard.admin instead of reviewboard.site (and fix paths).
Diff: |
Revision 3 (+54 -1) |
|---|