-
-
-
reviewboard/settings.py (Diff revision 1) 'from settings_local import *' used; unable to detect undefined names
Support managing OAuth2 applications in the db
Review Request #8630 — Created Jan. 17, 2017 and submitted
ReviewBoard now depends on
django-oauth-toolkitto provide support as
an OAuth2 identity provider for OAuth2 client applications. This first
patch allows applications to be managed in the database by
administrators. We use our own custom application model to allow
applications to be limited toLocalSites, as well as for attaching
extra data fields to them.This patch also provides help text for the application form fields, most
of which were missing them.
I was able to create an application in the database successfully.
| Description | From | Last Updated |
|---|---|---|
|
'django_reset' imported but unused |
 reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
'RelatedUserWidget' imported but unused |
reviewbot
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
Do we need to limit the version here to ensure compatibility with Django 1.6.x? |
 david
|
|
|
Class docstring? |
david
|
|
|
This is kind of hard to read. Can we do all the help text on the next line and indented … |
david
|
|
|
d before o. |
david
|
|
|
We should have _('Local Site') as the first argument to this to provide a localizable name in the admin. |
david
|
|
|
Same here with localizable name. |
david
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
'override_feature_check' imported but unused |
reviewbot
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
Col: 43 W291 trailing whitespace |
reviewbot
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
Col: 1 W391 blank line at end of file |
reviewbot
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
Formatting has to happen outside of _() |
david
|
|
|
As of Django 1.7, this needs to define either fields or excludes. To match the behavior you have here, use … |
david
|
|
|
Please add a Meta here that defines app_label, db_table, and has localized versions of verbose_name and verbose_name_plural |
david
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
Should be indented 2 spaces. |
david
|
|
|
This must also return self.cleaned_data. |
 chipx86
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
"but it" -> "but" |
chipx86
|
|
|
"the are" -> "they are" |
chipx86
|
|
|
"users' passwords" |
chipx86
|
|
|
Rather than space-separated, can we include a textarea and make it line-separated? |
chipx86
|
|
|
Probably want to say "unchecked" instead of "false", I would guess. |
chipx86
|
|
|
"is a" -> "is" |
chipx86
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
Blank line between these. |
chipx86
|
|
|
Should include the name of the block. Makes it easier to see in larger templates. |
chipx86
|
|
|
Missing "Returns." |
chipx86
|
|
|
Col: 80 E501 line too long (88 > 79 characters) |
reviewbot
|
|
|
You can wrap before the <. It's valid. |
chipx86
|
|
|
Minor thing, but technically you should get cleaned_data from this, modify that, and return it directly, instead of using self.cleaned_data. … |
chipx86
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
File doc comment? |
david
|
|
|
This should use ugettext instead of ugettext_lazy. |
david
|
|
|
Localized field name? |
david
|
|
|
Having random blank lines in the middle of this list when there aren't comments is weird. Can we clean that … |
david
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
Blank line after this. |
david
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
-
Tool: PEP8 Style Checker Processed Files: reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/settings.py reviewboard/oauth/forms.py Ignored Files: reviewboard/oauth/__init__.py Tool: Pyflakes Processed Files: reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/settings.py reviewboard/oauth/forms.py Ignored Files: reviewboard/oauth/__init__.py -
-
-
reviewboard/settings.py (Diff revision 2) 'from settings_local import *' used; unable to detect undefined names
-
Tool: Pyflakes Processed Files: reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/settings.py reviewboard/oauth/forms.py Ignored Files: reviewboard/oauth/__init__.py Tool: PEP8 Style Checker Processed Files: reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/settings.py reviewboard/oauth/forms.py Ignored Files: reviewboard/oauth/__init__.py -
-
reviewboard/settings.py (Diff revision 3) 'from settings_local import *' used; unable to detect undefined names
-
-
reviewboard/dependencies.py (Diff revision 3) Do we need to limit the version here to ensure compatibility with Django 1.6.x?
-
-
reviewboard/oauth/forms.py (Diff revision 3) This is kind of hard to read. Can we do all the help text on the next line and indented 4 spaces?
-
-
reviewboard/oauth/models.py (Diff revision 3) We should have
_('Local Site')as the first argument to this to provide a localizable name in the admin. -
Change Summary:
- Address David's issues.
- Add a change_list template that warns when oauth applications are not enabled (and therefore their owning user cannot modify them).
Diff: |
Revision 4 (+174 -29) |
|---|
-
Tool: Pyflakes Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py reviewboard/oauth/tests.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less Tool: PEP8 Style Checker Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py reviewboard/oauth/tests.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less -
-
-
Tool: PEP8 Style Checker Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less Tool: Pyflakes Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less -
-
Tool: PEP8 Style Checker Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less Tool: Pyflakes Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less -
-
-
Tool: Pyflakes Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less Tool: PEP8 Style Checker Processed Files: reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less -
-
Tool: Pyflakes Processed Files: reviewboard/settings.py reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/oauth/__init__.py reviewboard/static/rb/css/pages/admin.less Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/oauth/__init__.py reviewboard/static/rb/css/pages/admin.less -
-
-
-
reviewboard/settings.py (Diff revision 8) 'from settings_local import *' used; unable to detect undefined names
-
Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/oauth/__init__.py reviewboard/static/rb/css/pages/admin.less Tool: Pyflakes Processed Files: reviewboard/settings.py reviewboard/oauth/forms.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/features/checkers.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/oauth/__init__.py reviewboard/static/rb/css/pages/admin.less -
-
-
reviewboard/settings.py (Diff revision 9) 'from settings_local import *' used; unable to detect undefined names
-
Tool: Pyflakes Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/oauth/__init__.py reviewboard/static/rb/css/pages/admin.less Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/oauth/__init__.py reviewboard/static/rb/css/pages/admin.less -
-
-
reviewboard/settings.py (Diff revision 10) 'from settings_local import *' used; unable to detect undefined names
-
Because we're introducing a new app, we should simultaneously introduce an AppConfig for django 1.7+. The apps in djblets 0.10 have examples.
-
-
reviewboard/oauth/forms.py (Diff revision 10) As of Django 1.7, this needs to define either
fieldsorexcludes. To match the behavior you have here, usefields = '__all__' -
reviewboard/oauth/models.py (Diff revision 10) Please add a
Metahere that definesapp_label,db_table, and has localized versions ofverbose_nameandverbose_name_plural -
-
Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less Tool: Pyflakes Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py Ignored Files: reviewboard/templates/admin/oauth/application/change_list.html reviewboard/static/rb/css/pages/admin.less -
-
-
reviewboard/settings.py (Diff revision 11) 'from settings_local import *' used; unable to detect undefined names
-
This looks great, Barret! Thanks, and sorry it took so long to get around to it.
My comments just concern typos and very trivial things.
-
-
-
-
-
reviewboard/oauth/forms.py (Diff revision 11) Rather than space-separated, can we include a textarea and make it line-separated?
-
reviewboard/oauth/forms.py (Diff revision 11) Probably want to say "unchecked" instead of "false", I would guess.
-
-
reviewboard/templates/admin/oauth/application/change_list.html (Diff revision 11) Blank line between these.
-
reviewboard/templates/admin/oauth/application/change_list.html (Diff revision 11) Should include the name of the block. Makes it easier to see in larger templates.
-
Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py reviewboard/oauth/apps.py Tool: Pyflakes Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py reviewboard/oauth/apps.py -
-
-
reviewboard/settings.py (Diff revision 12) 'from settings_local import *' used; unable to detect undefined names
-
Trivial stuff. I'm happy after this.
-
-
-
reviewboard/oauth/forms.py (Diff revision 12) Minor thing, but technically you should get
cleaned_datafrom this, modify that, and return it directly, instead of usingself.cleaned_data. In practice, it doesn't matter really, since it's set by the baseForm.clean(), but worth doing.
-
Tool: Pyflakes Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py reviewboard/oauth/apps.py Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py reviewboard/oauth/apps.py -
-
reviewboard/settings.py (Diff revision 13) 'from settings_local import *' used; unable to detect undefined names
-
-
-
-
-
reviewboard/settings.py (Diff revision 13) Having random blank lines in the middle of this list when there aren't comments is weird. Can we clean that up?
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 14 (+272 -1) |
-
Tool: Pyflakes Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py reviewboard/oauth/apps.py Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/dependencies.py reviewboard/oauth/__init__.py reviewboard/oauth/models.py reviewboard/oauth/features.py reviewboard/oauth/forms.py reviewboard/oauth/admin.py reviewboard/oauth/apps.py -
-
reviewboard/settings.py (Diff revision 14) 'from settings_local import *' used; unable to detect undefined names
Tool: PEP8 Style Checker Processed Files: reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/settings.py reviewboard/oauth/forms.py Ignored Files: reviewboard/oauth/__init__.py Tool: Pyflakes Processed Files: reviewboard/dependencies.py reviewboard/oauth/admin.py reviewboard/oauth/models.py reviewboard/settings.py reviewboard/oauth/forms.py Ignored Files: reviewboard/oauth/__init__.py