RB Close
Review Request #3595 — Created Nov. 30, 2012 and submitted
RB Close Allows the user to submit a command 'rb close <rid>' to mark a review request as either 'discarded' or 'submitted'.
On local dev server, tried several combinations of the command and viewed the review request to ensure it match what was desired by the command. Working cases: rb close <rid> rb close <rid> --description='some desc' rb close <rid> --type='submitted' rb close <rid> --type='submitted' --description='some desc' rb close <rid> --type='discarded' rb close <rid> --type='discarded' --description='some desc' Bad input cases: rb close <bad rid> rb close <rid> --type='bad type' Retested with rb-general changes (removal of extra options)
Description | From | Last Updated |
---|---|---|
@smacleod : can we rip this out so I don't have to deal with all the coupling related to self.tool … |
JS jsintal | |
Any mentors know how to properly format this line? |
JS jsintal | |
"discarded" |
|
|
"user can provide" |
|
|
Shouldn't be needed. |
|
|
One line, and "number" isn't right. Just say "Returns the review request resource for the given ID." |
|
|
Where does this come from? I think you mean: sys.exit(1) |
|
|
type_ is kind of weird. How about close_type? |
|
|
A nice Pythonism you can use instead is: if close_type not in (SUBMITTED, DISCARDED): |
|
|
Should be a period after "type" |
|
|
close_type |
|
|
No need for parens. |
|
|
No need for parens. |
|
|
Slight alignment problem here I think: Either request.update(data={'status': type_, 'description': self.options.description}) Or request.update( data={ 'status': type_, 'description': self.options.description } ) … |
SL slchen | |
Alphabetical. |
|
|
lowercase the O, I believe. |
|
|
I'm starting to see this pattern repeat itself. Are we sure this isn't something we need in the parent class? |
|
|
Nit: indented too far |
|
-
-
rbtools/commands/rbclose.py (Diff revision 2) @smacleod : can we rip this out so I don't have to deal with all the coupling related to self.tool (SCM stuff)?
Change Summary:
Reinserted SCM related stuff (superfluous options, methods, and etc) for the scan_server_url() method so it doens't crash if it goes if it tries to use the SCM tool.
Diff: |
Revision 3 (+151 -43) |
---|
Change Summary:
- refactored according to the changes in base commadnd - pep8 style fixes - logic added to detect if a request had been already marked as 'discarded' or 'submitted'
Diff: |
Revision 4 (+116 -43) |
---|
-
-
rbtools/commands/rbclose.py (Diff revisions 3 - 4) Any mentors know how to properly format this line?
Change Summary:
Changed the way commands are branched from other features (rb-general). Ex.: http://pastie.org/5468764
Diff: |
Revision 5 (+1) |
---|
Change Summary:
rebased
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+81 -44) |
Change Summary:
Rebased on using the gettatr(options, field, default method for ALL
Diff: |
Revision 7 (+81 -44) |
---|
-
-
rbtools/commands/rbclose.py (Diff revision 7) Slight alignment problem here I think: Either request.update(data={'status': type_, 'description': self.options.description}) Or request.update( data={ 'status': type_, 'description': self.options.description } ) ?
-
-
-
-
-
rbtools/commands/rbclose.py (Diff revision 7) One line, and "number" isn't right. Just say "Returns the review request resource for the given ID."
-
rbtools/commands/rbclose.py (Diff revision 7) Where does this come from? I think you mean: sys.exit(1)
-
-
rbtools/commands/rbclose.py (Diff revision 7) A nice Pythonism you can use instead is: if close_type not in (SUBMITTED, DISCARDED):
-
-
-
-
-
-
-
-
rbtools/commands/rbclose.py (Diff revision 8) I'm starting to see this pattern repeat itself. Are we sure this isn't something we need in the parent class?
-
-
setup.py (Diff revision 8) Y'know, when all of these commands land, this is all gonna conflict right here. :)