Summary

Convert URL patterns to lists.

Review Request #8723 — Created Feb. 7, 2017 and submitted April 27, 2017, 1:27 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

c2417de...

Reviewers

Groups

reviewboard

People

Description

Django has deprecated the use of the old patterns() method, since they now

recommend that all URLs pass in the view directly rather than do string magic

to get the module path. This change moves us over to using plain lists of

url() entries, rather than a bunch of calls to patterns().
While I was doing this, I cleaned things up a bit with the regexes (such as

converting [0-9] to \d and [a-zA-Z0-9_] to \w), and changed it so we

only use raw strings where necessary.

Testing Done

Ran unit tests on Django 1.6 and 1.8.

Issues

Description	From	Last Updated
Instead of importing each view directly, I'm wondering about just importing views as appname_views (reviews_views, admin_views, etc.). Then we can …	chipx86	Feb. 9, 2017, 10:04 a.m.
One blank line, for consistency with the above.	chipx86	Feb. 15, 2017, 3:46 p.m.
I don't love having to mix these throughout the file, but I also never quite loved some of these being …	chipx86	Feb. 15, 2017, 6:07 p.m.
Mind removing these blank lines before except while here?	chipx86	Feb. 15, 2017, 3:46 p.m.
I'd prefer to keep the r prefix for all regexes. It makes it a lot easier to copy/paste/modify regexes without …	chipx86	Feb. 15, 2017, 6:07 p.m.
Swap these.	chipx86	Feb. 15, 2017, 3:47 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 15, 2017, 6:25 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 15, 2017, 6:23 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 15, 2017, 6:23 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 15, 2017, 6:23 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 15, 2017, 6:24 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 15, 2017, 6:24 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py

reviewboard/urls.py (Diff revision 1)

The issue has been resolved. Show all issues

Instead of importing each view directly, I'm wondering about just importing views as appname_views (reviews_views, admin_views, etc.). Then we can reference the particular views as attributes instead of managing the import lists. I've seen this pattern with some other Django apps.
What are your thoughts?

Commit:

c3acbd15e2f2eb9baa0f66c71c531ed60851a1bb

1824774d43391918d2250781b44306379426b76c

Diff:

Revision 2 (+490 -464)

Show changes

	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/admin/urls.py
	reviewboard/datagrids/urls.py
	reviewboard/hostingsvcs/beanstalk.py
	reviewboard/hostingsvcs/bitbucket.py
	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/googlecode.py
	6 more

Tool: Pyflakes
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py

Ship it!

```
Ship It!
```

reviewboard/datagrids/urls.py (Diff revision 2)
The issue has been resolved. Show all issues
```
One blank line, for consistency with the above.
```

reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)

The issue has been resolved. Show all issues

I don't love having to mix these throughout the file, but I also never quite loved some of these being top-level to begin with. Can we maybe introduce a BeanstalkHookViews class to contain this function and utility functions? Will also help with organization down the road when we add more hooks.
Same would apply to the other services.

reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)

The issue has been resolved. Show all issues

Mind removing these blank lines before except while here?

reviewboard/reviews/urls.py (Diff revision 2)

The issue has been resolved. Show all issues

I'd prefer to keep the r prefix for all regexes. It makes it a lot easier to copy/paste/modify regexes without unintentionally breaking things.

reviewboard/scmtools/admin.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Swap these.
```

Commit:

1824774d43391918d2250781b44306379426b76c

7573f60b1a6cf1913102e4affa06f1cafa64e424

Diff:

Revision 3 (+744 -572)

Show changes

	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/admin/urls.py
	reviewboard/datagrids/urls.py
	reviewboard/hostingsvcs/beanstalk.py
	reviewboard/hostingsvcs/bitbucket.py
	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/googlecode.py
	6 more

Tool: Pyflakes
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py

reviewboard/hostingsvcs/beanstalk.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/hostingsvcs/github.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/hostingsvcs/github.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/hostingsvcs/googlecode.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/hostingsvcs/googlecode.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/urls.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```

Commit:

7573f60b1a6cf1913102e4affa06f1cafa64e424

c2417de7f3e3fe920d7cd52e17527e11a09f9624

Diff:

Revision 4 (+750 -572)

Show changes

	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/admin/urls.py
	reviewboard/datagrids/urls.py
	reviewboard/hostingsvcs/beanstalk.py
	reviewboard/hostingsvcs/bitbucket.py
	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/googlecode.py
	6 more

Tool: Pyflakes
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/scmtools/admin.py
    reviewboard/hostingsvcs/urls.py
    reviewboard/hostingsvcs/beanstalk.py
    reviewboard/accounts/urls.py
    reviewboard/reviews/urls.py
    reviewboard/hostingsvcs/bitbucket.py
    reviewboard/admin/urls.py
    reviewboard/urls.py
    reviewboard/hostingsvcs/service.py
    reviewboard/search/urls.py
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/googlecode.py
    reviewboard/hostingsvcs/tests/test_registration.py
    reviewboard/datagrids/urls.py

Status:: Completed
Change Summary:: Pushed to release-3.0.x (151c59d6b).