Summary

Implement guessing for submitted-revision posting in SVN

Review Request #9413 — Created Nov. 30, 2017 and discarded Jan. 10, 2018, 4:19 p.m.

Information

Owner

GrahamS

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools, students

People

Description

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.

Testing Done

Manual Testing done.

Added unit test

ran nosetests -v; all tests passed.

Issues

Description	From	Last Updated
Setting commit = '' twice seems redundant here, but I might be missing something.	RC rcreagha	Dec. 8, 2017, 1:07 p.m.
Single quote '--rbtools-working-copy'	TB tbrockma	Dec. 10, 2017, 9:01 p.m.
Similarly, I believe here you can also use self.assertIsNone(commit)	TB tbrockma	Dec. 10, 2017, 9:01 p.m.
Might be better to use self.assertIn(collection, value) just so the purpose of the assertions are a bit more clear. ... …	TB tbrockma	Dec. 10, 2017, 9:01 p.m.
This should probably be: if (revisions['tip'] != SVNClient.REVISION_WORKING_COPY and not revisions['tip'].startswith( SVNClient.REVISION_CHANGELIST_PREFIX)):	david	Dec. 19, 2017, 12:05 p.m.
A couple small changes in here: desc -> description commited -> committed	david	Dec. 19, 2017, 12:05 p.m.
Can we add some blank lines in here to break things up? Or even split these two into separate test …	david	Dec. 19, 2017, 12:05 p.m.

flake8 passed.

JSHint passed.

Change Summary:

Added unit test for function.

Summary:

[WIP] Implement guessing for submitted-revision posting in SVN

Implement guessing for submitted-revision posting in SVN

Testing Done:

~		Still have to do automatic. Have done manual and it's working.
	~	Manual Testing done.
	+
	+	Added unit test
	+
	+	ran nosetests -v; all tests passed.

Commit:

801f88b56300ab633396850cee03fc1152bf4a4a

e393de25fa23734cef6e194fa91ebee402e21c6d

Diff:

Revision 2 (+39)

Show changes

	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/clients/svn.py (Diff revision 2)

The issue has been resolved. Show all issues

Setting commit = '' twice seems redundant here, but I might be missing something.

Change Summary:

Fixed double variable initalization.

Commit:

e393de25fa23734cef6e194fa91ebee402e21c6d

48d6114c91700741d538b1759a384488506279f5

Diff:

Revision 3 (+37)

Show changes

	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/clients/svn.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Single quote '--rbtools-working-copy'
```

rbtools/clients/tests/test_svn.py (Diff revision 3)

The issue has been resolved. Show all issues

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

rbtools/clients/tests/test_svn.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Similarly, I believe here you can also use self.assertIsNone(commit)
```

Change Summary:

Syntax changes to be more clear, based on Theodore's insightful review

Commit:

48d6114c91700741d538b1759a384488506279f5

aed4ad9be8ac19cb77dd519726f723a3d94bc2ac

Diff:

Revision 4 (+37)

Show changes

	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/clients/svn.py (Diff revision 4)

The issue has been resolved. Show all issues

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)
The issue has been resolved. Show all issues
```
A couple small changes in here:
desc -> description

commited -> committed
```

rbtools/clients/tests/test_svn.py (Diff revision 4)

The issue has been resolved. Show all issues

Can we add some blank lines in here to break things up? Or even split these two into separate test cases?

Change Summary:

Implemented changes and fixes based on reviews.

Commit:

aed4ad9be8ac19cb77dd519726f723a3d94bc2ac

476ecdae2bb21975b71183ce8277f5e0a8d2a1ca

Diff:

Revision 5 (+42)

Show changes

	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Status:: Discarded
Change Summary:: Discarded in favor of https://reviews.reviewboard.org/r/9478/