flake8
-
reviewboard/reviews/views/mixins.py (Diff revision 1) Show all issues
Review Request #12685 — Created Oct. 17, 2022 and submitted
Information | |
---|---|
david | |
Review Board | |
release-6.x | |
Reviewers | |
reviewboard | |
This change adds type hints to the reviews.views module, and does some
updates to documentation (filling out a few missing docstrings and
editing others).
Ran unit tests.
Summary | |
---|---|
Description | From | Last Updated |
---|---|---|
'typing.Dict' imported but unused Column: 1 Error code: F401 |
![]() |
|
Blank line between these. |
|
|
Missing Type: |
|
|
SafeString should precede mark_safe here. |
|
|
Blank line here. |
|
|
These will need to specify the actual module path to link up right. |
|
|
The description should be separated from the summary. |
|
|
We should probably have a * in here somewhere to separate positional from keyword-only arguments, to keep this maintainable. There … |
|
|
Same note about the module path. |
|
|
This will also need to specify the module path. Wish we had some automatic review that could catch these. We'll … |
|
|
line too long (82 > 79 characters) Column: 80 Error code: E501 |
![]() |
|
unexpected spaces around keyword / parameter equals Column: 31 Error code: E251 |
![]() |
|
Question: Since this is optional do we have to say datetime.datetime or None for the type? |
![]() |
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+1070 -320) |
Fantastic cleanup!
A few small things stood out, but nothing major.
reviewboard/reviews/views/bug_trackers.py (Diff revision 2) |
---|
SafeString
should precedemark_safe
here.
reviewboard/reviews/views/diff_fragments.py (Diff revision 2) |
---|
These will need to specify the actual module path to link up right.
reviewboard/reviews/views/diff_fragments.py (Diff revision 2) |
---|
The description should be separated from the summary.
reviewboard/reviews/views/diff_fragments.py (Diff revision 2) |
---|
We should probably have a
*
in here somewhere to separate positional from keyword-only arguments, to keep this maintainable. There might be other similar methods that could benefit.
reviewboard/reviews/views/diff_fragments.py (Diff revision 2) |
---|
This will also need to specify the module path.
Wish we had some automatic review that could catch these. We'll have to someday get automatic doc building as part of the tests.
Can you build docs and see if anything comes up for any of these?
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+1100 -332) |
reviewboard/notifications/email/message.py (Diff revision 3) |
---|
line too long (82 > 79 characters) Column: 80 Error code: E501
reviewboard/notifications/email/message.py (Diff revision 3) |
---|
unexpected spaces around keyword / parameter equals Column: 31 Error code: E251
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+1102 -332) |
reviewboard/reviews/views/mixins.py (Diff revision 4) |
---|
Question: Since this is optional do we have to say
datetime.datetime or None
for the type?