flake8
passed.
JSHint
passed.
Implement guessing for submitted-revision posting in SVN
Review Request #9413 — Created Nov. 30, 2017 and discarded
Added support for guessing submitted revision posting in SVN. The summary and
description of a review request are populated with the commit message from a
submitted revision in SVN.
Manual Testing done.
Added unit test
ran nosetests -v; all tests passed.
| Description | From | Last Updated |
|---|---|---|
|
Setting commit = '' twice seems redundant here, but I might be missing something. |
RC rcreagha | |
|
Single quote '--rbtools-working-copy' |
TB tbrockma | |
|
Similarly, I believe here you can also use self.assertIsNone(commit) |
TB tbrockma | |
|
Might be better to use self.assertIn(collection, value) just so the purpose of the assertions are a bit more clear. ... … |
TB tbrockma | |
|
This should probably be: if (revisions['tip'] != SVNClient.REVISION_WORKING_COPY and not revisions['tip'].startswith( SVNClient.REVISION_CHANGELIST_PREFIX)): |
 david
|
|
|
A couple small changes in here: desc -> description commited -> committed |
david
|
|
|
Can we add some blank lines in here to break things up? Or even split these two into separate test … |
david
|
GR
Change Summary:
Added unit test for function.
Summary: |
|
||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||
Commit: |
|
||||||||||||||||||
Diff: |
Revision 2 (+39) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
RC
-
-
rbtools/clients/svn.py (Diff revision 2) Setting
commit = ''twice seems redundant here, but I might be missing something.
GR
Change Summary:
Fixed double variable initalization.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+37) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
TB
-
-
rbtools/clients/tests/test_svn.py (Diff revision 3) Might be better to use self.assertIn(collection, value) just so the purpose of the assertions are a bit more clear.
... I only mention it because Barret gave me some issues in the past for the same thing. :P
TB
-
-
rbtools/clients/tests/test_svn.py (Diff revision 3) Similarly, I believe here you can also use self.assertIsNone(commit)
GR
Change Summary:
Syntax changes to be more clear, based on Theodore's insightful review
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 4 (+37) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
-
rbtools/clients/svn.py (Diff revision 4) This should probably be:
if (revisions['tip'] != SVNClient.REVISION_WORKING_COPY and not revisions['tip'].startswith( SVNClient.REVISION_CHANGELIST_PREFIX)): -
rbtools/clients/tests/test_svn.py (Diff revision 4) A couple small changes in here:
desc -> description
commited -> committed -
rbtools/clients/tests/test_svn.py (Diff revision 4) Can we add some blank lines in here to break things up? Or even split these two into separate test cases?
GR
Change Summary:
Implemented changes and fixes based on reviews.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 5 (+42) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.