-
-
reviewboard/reviews/evolutions/add_stages.py (Diff revision 1) Col: 80 E501 line too long (85 > 79 characters)
-
reviewboard/reviews/evolutions/add_stages.py (Diff revision 1) Col: 80 E501 line too long (89 > 79 characters)
-
reviewboard/reviews/evolutions/add_stages.py (Diff revision 1) Col: 1 W391 blank line at end of file
Add Stage field to a review request
Review Request #8495 — Created Oct. 26, 2016 and discarded
This feature will add a stage field to a review request that will provide some
context as to where a review request is in its development/review lifecycle.
This field provides visual information that will aid decision making on a
review request like whether to perform an automatic code review or wait for
the author to make changes.This feature adds built-in stages which are
- ApprovedStage
- CompletedStage
- DiscardedStage
- PendingChangesStage
- PendingReviewsStage
- UnpublishedStage
- WIPStage
All tests pass
Description | From | Last Updated |
---|---|---|
This is looking pretty solid. You will probably want to move on to writing unit tests. |
|
|
Missing trailing commma. |
|
|
Col: 80 E501 line too long (85 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (89 > 79 characters) |
![]() |
|
Col: 1 W391 blank line at end of file |
![]() |
|
Single quotes. |
|
|
Use the class itself, UnpublishedStage, instead of the instance. |
|
|
This should be formatted like a docstring, i.e. #: Single line summary #: #: Multi-line description should_review = False |
|
|
Move to builtin_stages.py |
|
|
MOve this to __init__.py |
|
|
This is unnecessary becuase of get_defaults |
|
|
Move to builtin_stages.py |
|
|
Move to builtin_stages.py |
|
|
Move to builtin_stages.py |
|
|
Move to builtin_stages.py |
|
|
Move to builtin_stages.py |
|
|
Move to builtin_stages.py |
|
|
Two blank lines between these. Missing a file docstring. Missing from __future__ import unicode literals. |
|
|
Not sure why this is here. |
|
|
Misisng file-level docstring. Missing from __future__ import unicode literals. |
|
|
Docstring. |
|
|
Dont need spacing between these. Same below. |
|
|
Docstring for all these. |
|
|
Misisng file-level docstring. Missing from __future__ import unicode literals. |
|
|
Docstring. |
|
|
Misisng file-level docstring. Missing from __future__ import unicode literals. |
|
|
Col: 38 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 80 E501 line too long (80 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (87 > 79 characters) |
![]() |
|
Args/kwargs missing. |
|
|
You also need a hook to add stages from extensions. See https://reviews.reviewboard.org/r/8530/ and https://reviews.reviewboard.org/r/8531/. |
|
|
The first import should be from __future__ import unicode_literals. |
|
|
Blank line between these. |
|
|
"""The global stage registry.""" |
|
|
stages Also, could you give a quick doc-comment before this (like #: ...). |
|
|
This needs to explain the contents of the file. |
|
|
Please elaborate on these docstrings, such as when they will be applied to review requests. |
|
|
This should be localized: from django.utils.translation import ugettext_lazy as _ # ... class UnpublishedStage(BaseReviewRequestStage): name = _('Unpublished') # ... Same … |
|
|
Wrong docstring. |
|
|
Please use proper capitilization and punctuation. |
|
|
Docstring. |
|
|
You're missing reply_published. Also, how about: ```python signals = ( (review_request_published, self._on_review_request_published), (review_request_closed, self._on_review_request_closed), (review_request_reopened, self._on_review_request_reopened), (review_published, self._on_review_published), (reply_published, self._on_reply_published), … |
|
|
_on_review_request_published. |
|
|
review_request_published will be triggered whenever the review request is published, so this may happen more than once. The first publish … |
|
|
Col: 38 E128 continuation line under-indented for visual indent |
![]() |
|
This needs to explain the contents of the file. |
|
|
Can you explain what this is. |
|
|
Can you add a get_stage(stage_id) method? |
|
|
Missing docstring. |
|
|
You can just return a list here. |
|
|
This needs to explain the contents of the file. |
|
|
Col: 80 E501 line too long (80 > 79 characters) |
![]() |
|
Col: 45 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 29 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 1 W391 blank line at end of file |
![]() |
|
If review.request.stage returns None, this will raise an AttributeError. |
|
|
Col: 80 E501 line too long (95 > 79 characters) |
![]() |
|
missing **kwargs in Args: |
|
|
Missing file level docstring. |
|
|
Docstring. |
|
|
Missing docstring. |
|
|
Should this perhaps compute the stage of the review request if self._stage is None? |
|
|
These all need translation. |
|
|
missing a space |
|
|
"completed" |
|
|
Docstrings here and elsewhere. |
|
|
No blank line |
|
|
Should we perhaps do this before calling self._calculate_review_request_stage? If a hook returns a stage, we throw the value computed by … |
|
|
('_stage',) Using a tuple requires less memory allocation. |
|
|
Missing period. If this is a generator this should be "Yield the registered stage classes." If this is not it … |
|
|
You can just return this instead of yielding. |
|
|
Use the full import path. |
|
|
Blank line between these. |
|
|
django.utils.safestring.SafeText |
|
|
No blank line. |
|
|
This is indented too far. And also incorrect. These are just extra arguments for the signal handler. |
|
|
No blank line. |
|
|
Two blank lines. |
|
|
Blank line. |
|
|
This should be reviewboard.reviews.stages.stages.BaseReviewRequestStage. |
|
|
Docstrings for all these. |
|
|
This is going to need many unit tests. |
|
|
No blank line. |
|
|
if not new_stage |
|
|
Should be :py:class:`.... Also the :py:class bit needs to be on the same line as the class bit. It is … |
|
|
No blank line. |
|
|
"for review requests." |
|
|
Missing Returns. |
|
|
Just return the tuple. |
|
|
Can you rename this to base.py? That's usually where we put base classes. |
|
|
Col: 27 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 80 E501 line too long (117 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (116 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (121 > 79 characters) |
![]() |
|
Col: 1 E303 too many blank lines (3) |
![]() |
|
Missing a period. Docstrings should be complete sentences. |
|
|
Should end in a period. |
|
|
Description should go on the next line. |
|
|
Should be imperative mood ("Return" instead of "Returns"), and needs a "Returns" section. |
|
|
Needs a period. |
|
|
You don't need the u prefix on the string literals because we import unicode_literals at the top. |
|
|
Can we alphabetize these? |
|
|
Should be the imperative mood ("Return ..." instead of "Returns ...") |
|
|
In "Returns", the description doesn't need to be indented relative to the type. |
|
|
Should be capitalized and imperative mood ("Set" instead of "sets") |
|
|
In "Returns", the description doesn't need to be indented relative to the type. |
|
|
Can we use the symbols instead of the string literals? In this case, instead of 'S', use ReviewRequest.SUBMITTED. |
|
|
Same here. |
|
|
Can review_request.status be something other than S, D and P? |
ST stensby | |
Same here. |
|
|
Add a period. |
|
|
Do you need this many comments for the variables? Most of them seem very self-explanatory. |
ST stensby | |
This description is a little circular. How about "The Review Request has not yet been published"? Also, do we want … |
|
|
"Waiting on a review" is a little confusing. Maybe "The author is waiting for reviews"? |
|
|
Maybe "Waiting for the author to make requested changes"? |
|
|
Description should go on the next line. |
|
|
Description should go on the next line. |
|
|
Description should go on the next line. |
|
|
This sentence doesn't really make sense. Maybe "Whether the review request is being published for the first time"? |
|
|
This sentence doesn't really make sense. Maybe "Whether the review request is being published for the first time"? |
|
|
In "Returns" sections, you don't need to indent the description relative to the type. |
|
|
Instead of using the character to continue the statement, please wrap the whole conditional in parentheses. We can also use … |
|
|
Alphabetize? |
|
|
This should be written in the imperative mood ("Return the registered...") and include a "Returns" section. |
|
|
Same here. Also needs to end in a period. |
|
|
Needs a period. |
|
|
Alphabetize? |
|
|
In test docstrings, we don't add the normal period at the end because the test suite adds "...". Here and … |
|
|
Should be two blank lines here. |
|
|
You don't need a blank line after docstrings for methods (only the docstring for classes). |
|
|
Mind adding another blank line here? |
|
|
It looks like there's some overlap with ReviewRequestStageController.calculate_review_request_stage? |
|
|
We should move this to BaseReviewRequestDetails too. |
|
|
Add a blank line here. |
|
|
"review request" should be sentence case here. Let's also add a period to the end of all of these. |
|
|
Let's make all of these consistent in their language: "The review request ...". Here and below. |
|
|
Dedent these 4 spaces. |
|
|
Here too. |
|
|
And here, and below. |
|
|
review_request.stage should be vertically aligned with first_publish. |
|
|
These can be one statement (just return the tuple) |
|
|
Missing docs on Args. |
|
|
No indentation here for the description. |
|
|
This should check for a value more generally, rather than None, since it can be ''. if review_request.stage: ... |
|
|
Let's also include this in the list of default columns. |
|
|
Should be reviewboard.extensions.base.Extension. |
|
|
There can't be a blank line here. Also, it must be :py:class:, not py:class: |
|
|
Docstrings are only valuable when they convey something the user doesn't already know. This just says what __init__.py is, generally. … |
|
|
I would expect this to live in reviewboard/reviews/stages/controller.py, with a setter and getter (like we have for other singleton instances). |
|
|
Needs a trailing comma. |
|
|
This should remain only in ReviewRequest. |
|
|
This should remain only in ReviewRequest. |
|
|
This should be a CharField, not a TextField. The latter is meant for larger pieces of text, and won't index … |
|
|
Args is indented too far. |
|
|
`:py:class: |
|
|
This function should go away. All computation is going to happen in the controller. ReviewRequest.__init__ will check if the stage … |
|
|
:py:class: |
|
|
This file is more than just the registry. It's the whole module for working with stages. This specific file just … |
|
|
Read this as if you weren't aware of the stages work, and then see if you can think of some … |
|
|
"built-in" |
|
|
No point in setting the values if they aren't changing from the base class. Can you go through each of … |
|
|
This should include some information on how those updates are applied, and how consumers might override computation. |
|
|
We usually say something like "Handler for review request published signals." |
|
|
This is the wrong module path. Same below. |
|
|
You can combine these into one call: self._update_review_request_stage( review_request, first_publish=changedesc is None) |
|
|
Should be (bool, optional) |
|
|
Let's use keyword arguments in the call. Helps for future expansion. |
|
|
Blank line between these. |
|
|
:py:class: |
|
|
We don't use title case. This should better summarize what this class is. "A registry for tracking available stages for … |
|
|
Missing period. This should also flesh out the responsibilities of this class a bit more. Remember that, for every docstring, … |
|
|
The docstring isn't correct. This isn't returning registered stages. It's returning the default stages that the registry knows about. |
|
|
A couple things here: The description is redundant. That's implied by the type. Don't just talk return values and data … |
|
|
:py:class: |
|
|
"... registered to the provided stage ID." |
|
|
As above, this should be talking about what's truly being returned beyond just a data type. |
|
|
A tests.py file should contain unit tests. Given the simplicity of this dummy object, I don't think we should bother … |
|
|
Test docstrings should mention the classes/attribute/method/etc. being worked with. However, these tests are all testing different classes, which is often … |
|
|
I wouldn't expect this call to be necessary? That should be calculated on access. |
|
|
Col: 1 W391 blank line at end of file |
![]() |
|
'DiscardedStage' imported but unused |
![]() |
|
'CompletedStage' imported but unused |
![]() |
|
'PendingReviewsStage' imported but unused |
![]() |
|
'UnpublishedStage' imported but unused |
![]() |
|
'WIPStage' imported but unused |
![]() |
|
Col: 1 W391 blank line at end of file |
![]() |
|
Col: 1 W391 blank line at end of file |
![]() |
|
'DiscardedStage' imported but unused |
![]() |
|
'CompletedStage' imported but unused |
![]() |
|
'PendingReviewsStage' imported but unused |
![]() |
|
'UnpublishedStage' imported but unused |
![]() |
|
'WIPStage' imported but unused |
![]() |
|
Col: 1 W391 blank line at end of file |
![]() |
|
This should actually be None initially. |
|
|
You need to do: from reviewboard.reviews.stages import stage_controller # ... stage_controller.calculate_review_request_stage(self.get_review_request()) |
|
|
This shouldn't be here. |
|
|
redefinition of unused 'TestCase' from line 55 |
![]() |

-
-
-
-
reviewboard/reviews/stages/registry.py (Diff revision 1) Use the class itself,
UnpublishedStage
, instead of the instance. -
reviewboard/reviews/stages/stage.py (Diff revision 1) This should be formatted like a docstring, i.e.
#: Single line summary #: #: Multi-line description should_review = False
-
-
-
-
-
-
-
-
-
Change Summary:
fixing stage folder issues

-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/stages/registry.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/stages/registry.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py
-
-
reviewboard/reviews/stages/__init__.py (Diff revision 2) Two blank lines between these.
Missing a file docstring.
Missingfrom __future__ import unicode literals
. -
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 2) Misisng file-level docstring.
Missingfrom __future__ import unicode literals
. -
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 2) Dont need spacing between these. Same below.
-
-
reviewboard/reviews/stages/registry.py (Diff revision 2) Misisng file-level docstring.
Missingfrom __future__ import unicode literals
. -
-
reviewboard/reviews/stages/stage.py (Diff revision 2) Misisng file-level docstring.
Missingfrom __future__ import unicode literals
.
Change Summary:
new controller chanegs and integration
Commit: |
|
||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+250)
|

-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/stages/registry.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/controller.py reviewboard/reviews/__init__.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py Ignored Files: reviewboard/templates/reviews/review_request_box.html Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/stages/registry.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/controller.py reviewboard/reviews/__init__.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py Ignored Files: reviewboard/templates/reviews/review_request_box.html
-
reviewboard/reviews/stages/controller.py (Diff revision 3) Col: 38 E128 continuation line under-indented for visual indent
-
reviewboard/reviews/stages/controller.py (Diff revision 3) Col: 80 E501 line too long (80 > 79 characters)
-
reviewboard/reviews/stages/controller.py (Diff revision 3) Col: 80 E501 line too long (87 > 79 characters)
Change Summary:
added UI option for stages and prilminary hook functionality
Commit: |
|
||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+313)
|

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py
-
reviewboard/reviews/stages/controller.py (Diff revision 4) Col: 38 E128 continuation line under-indented for visual indent
-
-
-
reviewboard/extensions/hooks.py (Diff revision 4) You also need a hook to add stages from extensions. See https://reviews.reviewboard.org/r/8530/ and https://reviews.reviewboard.org/r/8531/.
-
reviewboard/reviews/__init__.py (Diff revision 4) The first import should be
from __future__ import unicode_literals
. -
-
-
reviewboard/reviews/stages/__init__.py (Diff revision 4) stages
Also, could you give a quick doc-comment before this (like
#: ...
). -
reviewboard/reviews/stages/builtin_stages.py (Diff revision 4) This needs to explain the contents of the file.
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 4) Please elaborate on these docstrings, such as when they will be applied to review requests.
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 4) This should be localized:
from django.utils.translation import ugettext_lazy as _ # ... class UnpublishedStage(BaseReviewRequestStage): name = _('Unpublished') # ...
Same for all stages.
-
-
reviewboard/reviews/stages/controller.py (Diff revision 4) Please use proper capitilization and punctuation.
-
-
reviewboard/reviews/stages/controller.py (Diff revision 4) You're missing reply_published. Also, how about:
```python
signals = (
(review_request_published, self._on_review_request_published),
(review_request_closed, self._on_review_request_closed),
(review_request_reopened, self._on_review_request_reopened),
(review_published, self._on_review_published),
(reply_published, self._on_reply_published),
)for signal, handler in signals:
signal.connect(handler) -
-
reviewboard/reviews/stages/controller.py (Diff revision 4) review_request_published
will be triggered whenever the review request is published, so this may happen more than once. The first publish occurs whenchangedesc is None
, however. -
reviewboard/reviews/stages/registry.py (Diff revision 4) This needs to explain the contents of the file.
-
-
-
-
-
reviewboard/reviews/stages/stage.py (Diff revision 4) This needs to explain the contents of the file.
Change Summary:
adding stage hooks
Commit: |
|
||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+403)
|

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py
-
-
reviewboard/reviews/stages/controller.py (Diff revision 5) Col: 45 E127 continuation line over-indented for visual indent
-
reviewboard/reviews/stages/controller.py (Diff revision 5) Col: 29 E126 continuation line over-indented for hanging indent
-
Change Summary:
issue fixes and adding proper hook parent class
Commit: |
|
||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+403 -1)
|

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py
-
-
-
reviewboard/datagrids/columns.py (Diff revision 6) If
review.request.stage
returnsNone
, this will raise anAttributeError
. -
-
-
-
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 6) Should this perhaps compute the stage of the review request if
self._stage
isNone
? -
-
-
-
-
-
reviewboard/reviews/stages/controller.py (Diff revision 6) Should we perhaps do this before calling
self._calculate_review_request_stage
? If a hook returns a stage, we throw the value computed by_calculate_review_request_stage
out anyway. -
reviewboard/reviews/stages/controller.py (Diff revision 6) ('_stage',)
Using a tuple requires less memory allocation.
-
reviewboard/reviews/stages/registry.py (Diff revision 6) Missing period.
If this is a generator this should be "Yield the registered stage classes."
If this is not it should be "Return the registered stage classes."
Method docstrings should be in the imperitive mood, i.e., they instruct you as to what they are doing. They should never begin with "this".
-
reviewboard/reviews/stages/registry.py (Diff revision 6) You can just return this instead of yielding.
Change Summary:
fixing issues and working on the stage implementation
Commit: |
|
||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+498 -3)
|

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/stage.py reviewboard/reviews/models/base_review_request_details.py
-
-
-
-
-
-
-
reviewboard/reviews/__init__.py (Diff revision 7) This is indented too far. And also incorrect. These are just extra arguments for the signal handler.
-
-
-
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 7) This should be
reviewboard.reviews.stages.stages.BaseReviewRequestStage
. -
-
-
-
-
reviewboard/reviews/stages/controller.py (Diff revision 7) Should be
:py:class:`...
. Also the:py:class
bit needs to be on the same line as the class bit. It is okay if it goes over 80 chars for cases like this where it is unavaoidable. -
-
-
-
-
reviewboard/reviews/stages/stage.py (Diff revision 7) Can you rename this to
base.py
? That's usually where we put base classes.
Change Summary:
Adding unit test for controller and hooks, fixing review issues
Testing Done: |
|
||||||
---|---|---|---|---|---|---|---|
Commit: |
|
||||||
Diff: |
Revision 8 (+692 -3) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py
-
reviewboard/extensions/tests.py (Diff revision 8) Col: 27 E128 continuation line under-indented for visual indent
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 8) Col: 80 E501 line too long (117 > 79 characters)
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 8) Col: 80 E501 line too long (116 > 79 characters)
-
reviewboard/reviews/stages/controller.py (Diff revision 8) Col: 80 E501 line too long (121 > 79 characters)
-
Change Summary:
fixing bot issues
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 9 (+692 -3) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py
-
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 9) Can
review_request.status
be something other than S, D and P? -
reviewboard/reviews/stages/base.py (Diff revision 9) Do you need this many comments for the variables? Most of them seem very self-explanatory.
-
-
-
-
reviewboard/extensions/hooks.py (Diff revision 9) Should be imperative mood ("Return" instead of "Returns"), and needs a "Returns" section.
-
-
reviewboard/reviews/evolutions/add_stages.py (Diff revision 9) You don't need the
u
prefix on the string literals because we importunicode_literals
at the top. -
reviewboard/reviews/models/base_review_request_details.py (Diff revision 9) Can we alphabetize these?
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 9) Should be the imperative mood ("Return ..." instead of "Returns ...")
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 9) In "Returns", the description doesn't need to be indented relative to the type.
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 9) Should be capitalized and imperative mood ("Set" instead of "sets")
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 9) In "Returns", the description doesn't need to be indented relative to the type.
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 9) Can we use the symbols instead of the string literals? In this case, instead of
'S'
, useReviewRequest.SUBMITTED
. -
-
-
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 9) This description is a little circular. How about "The Review Request has not yet been published"?
Also, do we want to include periods in these strings?
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 9) "Waiting on a review" is a little confusing. Maybe "The author is waiting for reviews"?
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 9) Maybe "Waiting for the author to make requested changes"?
-
-
-
-
reviewboard/reviews/stages/controller.py (Diff revision 9) This sentence doesn't really make sense. Maybe "Whether the review request is being published for the first time"?
-
reviewboard/reviews/stages/controller.py (Diff revision 9) This sentence doesn't really make sense. Maybe "Whether the review request is being published for the first time"?
-
reviewboard/reviews/stages/controller.py (Diff revision 9) In "Returns" sections, you don't need to indent the description relative to the type.
-
reviewboard/reviews/stages/controller.py (Diff revision 9) Instead of using the character to continue the statement, please wrap the whole conditional in parentheses. We can also use the
in
operator to combine the second two clauses:if (first_publish and review_request.stage in (None, UnpublishedStage)):
-
-
reviewboard/reviews/stages/registry.py (Diff revision 9) This should be written in the imperative mood ("Return the registered...") and include a "Returns" section.
-
-
-
-
reviewboard/reviews/tests/test_stages.py (Diff revision 9) In test docstrings, we don't add the normal period at the end because the test suite adds "...". Here and all the other methods in here.
-
-
reviewboard/datagrids/columns.py (Diff revision 9) Missing a period. Docstrings should be complete sentences.
Change Summary:
fixing review changes mostly doc string corrections
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 10 (+718 -10) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py
Change Summary:
updating description
Description: |
|
---|
Change Summary:
fixing imports to be alphabetical
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 11 (+718 -10) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py
-
-
-
reviewboard/extensions/tests.py (Diff revision 11) You don't need a blank line after docstrings for methods (only the docstring for classes).
-
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 11) It looks like there's some overlap with
ReviewRequestStageController.calculate_review_request_stage
? -
reviewboard/reviews/models/review_request.py (Diff revision 11) We should move this to
BaseReviewRequestDetails
too. -
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 11) "review request" should be sentence case here. Let's also add a period to the end of all of these.
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 11) Let's make all of these consistent in their language: "The review request ...". Here and below.
-
-
-
-
reviewboard/reviews/stages/controller.py (Diff revision 11) review_request.stage
should be vertically aligned withfirst_publish
. -
reviewboard/reviews/stages/registry.py (Diff revision 11) These can be one statement (just return the tuple)
Change Summary:
fixing review issues on doc strings and a few other minor things
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 12 (+723 -15) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/stages/tests.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py
-
-
-
-
reviewboard/datagrids/columns.py (Diff revision 12) This should check for a value more generally, rather than
None
, since it can be''
.if review_request.stage: ...
-
reviewboard/datagrids/grids.py (Diff revision 12) Let's also include this in the list of default columns.
-
-
reviewboard/extensions/hooks.py (Diff revision 12) There can't be a blank line here.
Also, it must be
:py:class:
, notpy:class:
-
reviewboard/reviews/__init__.py (Diff revision 12) Docstrings are only valuable when they convey something the user doesn't already know. This just says what
__init__.py
is, generally. Instead, this should be explaining the purpose of this file. -
reviewboard/reviews/__init__.py (Diff revision 12) I would expect this to live in
reviewboard/reviews/stages/controller.py
, with a setter and getter (like we have for other singleton instances). -
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 12) This should remain only in
ReviewRequest
. -
reviewboard/reviews/models/base_review_request_details.py (Diff revision 12) This should remain only in
ReviewRequest
. -
reviewboard/reviews/models/base_review_request_details.py (Diff revision 12) This should be a
CharField
, not aTextField
. The latter is meant for larger pieces of text, and won't index as well.We also want to move this to
ReviewRequest
and toReviewRequestDraft
. The difference between the two is thatReviewRequest
's would have a default value for the Unpublished stage. This will apply to all brand new review requests (not existing ones from a legacy database). All review requests start out as Unpublished.ReviewRequestDraft
would not have a default value. The default would be set when the draft is created, based on the value on the review request. It's a copy, just like how we copy the summary, description, etc.The fields also need:
null=True
(so that we can have default, null values for existing database entries)max_length=64
(to leave room for vendor prefixes for third-party extensions)
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 12) Args
is indented too far. -
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 12) This function should go away. All computation is going to happen in the controller.
ReviewRequest.__init__
will check if the stage is empty and, if so, call out to the controller to set a value.ReviewRequestDraft.__init__
will do the same check, but set its own value from the review request. -
-
reviewboard/reviews/stages/__init__.py (Diff revision 12) This file is more than just the registry. It's the whole module for working with stages. This specific file just happens to contain the registry.
It'd be nice to have this go into a little more detail on what stages are.
-
reviewboard/reviews/stages/base.py (Diff revision 12) Read this as if you weren't aware of the stages work, and then see if you can think of some ways to flesh out the docstring.
-
-
reviewboard/reviews/stages/builtin_stages.py (Diff revision 12) No point in setting the values if they aren't changing from the base class. Can you go through each of these and make sure flags are only set if they differ in value?
-
reviewboard/reviews/stages/controller.py (Diff revision 12) This should include some information on how those updates are applied, and how consumers might override computation.
-
reviewboard/reviews/stages/controller.py (Diff revision 12) We usually say something like "Handler for review request published signals."
-
reviewboard/reviews/stages/controller.py (Diff revision 12) This is the wrong module path.
Same below.
-
reviewboard/reviews/stages/controller.py (Diff revision 12) You can combine these into one call:
self._update_review_request_stage( review_request, first_publish=changedesc is None)
-
-
reviewboard/reviews/stages/controller.py (Diff revision 12) Let's use keyword arguments in the call. Helps for future expansion.
-
-
-
reviewboard/reviews/stages/registry.py (Diff revision 12) We don't use title case. This should better summarize what this class is. "A registry for tracking available stages for a review request."
-
reviewboard/reviews/stages/registry.py (Diff revision 12) Missing period.
This should also flesh out the responsibilities of this class a bit more. Remember that, for every docstring, you should be trying to read this from the point of view of someone who has no idea what stages are, and the docstrings should educate on that.
-
reviewboard/reviews/stages/registry.py (Diff revision 12) The docstring isn't correct. This isn't returning registered stages. It's returning the default stages that the registry knows about.
-
reviewboard/reviews/stages/registry.py (Diff revision 12) A couple things here:
- The description is redundant. That's implied by the type. Don't just talk return values and data types. Talk about purpose. What is actually being returned?
- The data types are also wrong here. This is returning a tuple of default stages.
-
-
reviewboard/reviews/stages/registry.py (Diff revision 12) "... registered to the provided stage ID."
-
reviewboard/reviews/stages/registry.py (Diff revision 12) As above, this should be talking about what's truly being returned beyond just a data type.
-
reviewboard/reviews/stages/tests.py (Diff revision 12) A
tests.py
file should contain unit tests. Given the simplicity of this dummy object, I don't think we should bother keeping a One True Dummy Object. Instead, have each test suite define their own.The reason for this is that, over time, complexity grows, and tests need to test more variations. You can run the risk of someone changing this object and accidentally impacting other tests.
If you have a One True Dummy Object, you're going to eventually need 50 True Dummy Objects, which will all end up shoved into the same place, and it just creates a maintenance nightmare.
So let's nuke this file and instead define a dummy stage in the tests/files that need them.
-
reviewboard/reviews/tests/test_stages.py (Diff revision 12) Test docstrings should mention the classes/attribute/method/etc. being worked with. However, these tests are all testing different classes, which is often a red flag.
I think overall, this test suite is trying to do too many things. From the standpoint of your project, this all feels like "stage unit tests," but from the standpoint of the codebase, you're really testing different areas of responsibility.
What I'd suggest is:
- Having any tests involving default review request stage state living in
test_review_request.py
. - Any tests involving review request draft state (copying initial values from the review request) should live in
test_review_request_draft.py
. - Tests involving the controller specifically should live in
test_review_request_stage_controller.py
.
- Having any tests involving default review request stage state living in
-
reviewboard/reviews/tests/test_stages.py (Diff revision 12) I wouldn't expect this call to be necessary? That should be calculated on access.
Change Summary:
modifying implementation so that stage controller handles all computational logic
Branch: |
|
||||
---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 13 (+895 -1) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/reviews/tests/test_stages.py reviewboard/extensions/tests.py
-
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 13) 'DiscardedStage' imported but unused
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 13) 'CompletedStage' imported but unused
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 13) 'PendingReviewsStage' imported but unused
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 13) 'UnpublishedStage' imported but unused
-
reviewboard/reviews/tests/test_review_request_stage_controller.py (Diff revision 13) 'WIPStage' imported but unused
-
reviewboard/reviews/tests/test_review_request_stage_controller.py (Diff revision 13) Col: 1 W391 blank line at end of file
Change Summary:
deleting old test stages suite
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 14 (+775 -1) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py
-
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 14) 'DiscardedStage' imported but unused
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 14) 'CompletedStage' imported but unused
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 14) 'PendingReviewsStage' imported but unused
-
reviewboard/reviews/models/base_review_request_details.py (Diff revision 14) 'UnpublishedStage' imported but unused
-
reviewboard/reviews/tests/test_review_request_stage_controller.py (Diff revision 14) 'WIPStage' imported but unused
-
reviewboard/reviews/tests/test_review_request_stage_controller.py (Diff revision 14) Col: 1 W391 blank line at end of file
Change Summary:
fising bot issues
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 15 (+767 -1) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py
-
-
-
reviewboard/reviews/models/review_request.py (Diff revision 15) You need to do:
from reviewboard.reviews.stages import stage_controller # ... stage_controller.calculate_review_request_stage(self.get_review_request())
-
Branch: |
|
||||
---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 16 (+764 -1) |

-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py
-

-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py