flake8
-
rbintegrations/asana/fields.py (Diff revision 1) Show all issues -
-
Review Request #9363 — Created Nov. 11, 2017 and submitted
This change adds a new integration for linking to Asana tasks from
review requests. This consists of a few pieces: the integration config
which connects to a specific Asana workspace, and a review request field
which allows people to search for and link to multiple tasks.The UI is currently fully functional, but at a later date I'd like to
make it look a bit more like the related object selector used in the
admin UI, with the search box shown above the list of selected items
(instead of swapping out the entire UI for what looks more like "tags"
inside the selectize widget).
Description | From | Last Updated |
---|---|---|
I just mentioned this in the Trello change as well, but the one thing I want a better assurance of … |
|
|
F401 'django.utils.six' imported but unused |
![]() |
|
E501 line too long (130 > 79 characters) |
![]() |
|
E501 line too long (82 > 79 characters) |
![]() |
|
E501 line too long (130 > 79 characters) |
![]() |
|
Missing a docstring. |
|
|
Should be localized. |
|
|
Mind elaborating here on when it's rendered? |
|
|
You can simplify this a bit by starting off with: if not value: return '' Lines will be less crowded … |
|
|
Can we maybe use templates for rendering instead of hard-coding HTML here? |
|
|
E501 line too long (130 > 79 characters) |
![]() |
|
This isn't true for this function. |
|
|
Swap these. |
|
|
Should be localized. |
|
|
This is in the wrong import group. |
|
|
Missing a period. |
|
|
We should catch exceptions and log them so this doesn't crash if something goes wrong (network issues, data issues, whatever). |
|
|
This view should use CheckLoginRequiredViewMixin and CheckLocalSiteAccessViewMixin. |
|
|
Can you do: .asana-field { &.selectize-control.loading:before { ... } &.selectize-dropdown { ... } } |
|
|
Too many colons. Can we use a class instead? I have a change pending for post-3.0 that will get rid … |
|
|
IDs before classes. Same with others further down. |
|
|
Instead of hard-coding 13px, can we use an existing variable definition? |
|
|
Can we defein variables for the colors being used? Here and below. |
|
|
What do we need flex here for? It still doesn't have wide-enough working support. |
|
|
This can nest in #field_rbintegrations_asana above. |
|
|
Same comments here about double colons and using a span. |
|
|
No need for parens here. |
|
|
Looks like we're duplicating the one from the Python field. Can we standardize these somehow? |
|
|
Better as callback.bind(this). |
|
|
If you do: if (apiKey.length === 0) { return; } You can save an indentation level and reduce the line … |
|
|
You can simplify to: $accessToken.on('change keyup', changeWorkspaceEnabled); |
|
|
Alphabetical order (case-insensitive for dependencies). |
|
|
F401 'json' imported but unused |
![]() |
|
F401 'django.utils.html.format_html' imported but unused |
![]() |
|
F401 'django.utils.html.format_html_join' imported but unused |
![]() |
|
E501 line too long (82 > 79 characters) |
![]() |
|
Can we set a logger = ... above, and use that here and anywhere else we might be logging? |
|
|
The mixins must go first. |
|
|
F401 'kgb' imported but unused |
![]() |
rbintegrations/asana/fields.py (Diff revision 1) |
---|
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+781) |
Typo.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+781) |
rbintegrations/asana/fields.py (Diff revision 3) |
---|
You can simplify this a bit by starting off with:
if not value: return ''
Lines will be less crowded that way.
rbintegrations/asana/fields.py (Diff revision 3) |
---|
Can we maybe use templates for rendering instead of hard-coding HTML here?
rbintegrations/asana/views.py (Diff revision 3) |
---|
We should catch exceptions and log them so this doesn't crash if something goes wrong (network issues, data issues, whatever).
rbintegrations/asana/views.py (Diff revision 3) |
---|
This view should use
CheckLoginRequiredViewMixin
andCheckLocalSiteAccessViewMixin
.
rbintegrations/static/css/asana/asana.less (Diff revision 3) |
---|
Can you do:
.asana-field { &.selectize-control.loading:before { ... } &.selectize-dropdown { ... } }
rbintegrations/static/css/asana/asana.less (Diff revision 3) |
---|
Too many colons.
Can we use a class instead? I have a change pending for post-3.0 that will get rid of FontAwesome spinners (which are broken and officially deprecated in terms of being recommended) and replace them with a non-wobbly spinner. It will require a
<span>
of some sort, and it'll be easier for me to update this if we're not duplicating FontAwesome CSS here.
rbintegrations/static/css/asana/asana.less (Diff revision 3) |
---|
IDs before classes.
Same with others further down.
rbintegrations/static/css/asana/asana.less (Diff revision 3) |
---|
Instead of hard-coding 13px, can we use an existing variable definition?
rbintegrations/static/css/asana/asana.less (Diff revision 3) |
---|
Can we defein variables for the colors being used? Here and below.
rbintegrations/static/css/asana/asana.less (Diff revision 3) |
---|
What do we need flex here for? It still doesn't have wide-enough working support.
rbintegrations/static/css/asana/asana.less (Diff revision 3) |
---|
This can nest in
#field_rbintegrations_asana
above.
rbintegrations/static/css/asana/integration-config.less (Diff revision 3) |
---|
Same comments here about double colons and using a span.
rbintegrations/static/js/asana/asanaFieldView.es6.js (Diff revision 3) |
---|
Looks like we're duplicating the one from the Python field. Can we standardize these somehow?
rbintegrations/static/js/asana/asanaFieldView.es6.js (Diff revision 3) |
---|
Better as
callback.bind(this)
.
rbintegrations/static/js/asana/integrationConfig.es6.js (Diff revision 3) |
---|
If you do:
if (apiKey.length === 0) { return; }
You can save an indentation level and reduce the line widths for the rest of these.
rbintegrations/static/js/asana/integrationConfig.es6.js (Diff revision 3) |
---|
You can simplify to:
$accessToken.on('change keyup', changeWorkspaceEnabled);
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+778) |
rbintegrations/asana/fields.py (Diff revision 4) |
---|
F401 'django.utils.html.format_html' imported but unused
rbintegrations/asana/fields.py (Diff revision 4) |
---|
F401 'django.utils.html.format_html_join' imported but unused
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+776) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+779) |
rbintegrations/asana/views.py (Diff revision 6) |
---|
Can we set a
logger = ...
above, and use that here and anywhere else we might be logging?
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+876) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 8 (+875) |