Summary

Making it easy to write pre/post-commit hooks

Review Request #9236 — Created Sept. 29, 2017 and discarded Jan. 21, 2021, 2:43 p.m.

Information

Owner

GrahamS

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools, students

People

Description

Applies the 0001-WIP-Repository-hooks-support.patch that Christian Hammond
created.

Adds the repo-hook command to make it easy to use pre and post commit hooks. The
supported repositories are Git, Mercurial and SVN.

Testing Done

Ran nosetests -v; all tests pass with patch applied.

Created unit tests for all functions.

Ran nosetests -v; all tests passed.

Issues

Description	From	Last Updated
F401 'json' imported but unused	reviewbot	Oct. 7, 2017, 12:17 p.m.
F401 'six.moves.urllib.request.urlopen' imported but unused	reviewbot	Oct. 7, 2017, 12:18 p.m.
F401 'rbtools.api.tests.MockResponse' imported but unused	reviewbot	Oct. 7, 2017, 12:18 p.m.
F401 'rbtools.clients.svn.SVNClient' imported but unused	reviewbot	Oct. 7, 2017, 12:18 p.m.
E265 block comment should start with '# '	reviewbot	Oct. 13, 2017, 8:58 p.m.
E265 block comment should start with '# '	reviewbot	Oct. 13, 2017, 8:58 p.m.
E265 block comment should start with '# '	reviewbot	Oct. 13, 2017, 8:58 p.m.
E265 block comment should start with '# '	reviewbot	Oct. 13, 2017, 8:58 p.m.
F811 redefinition of unused 'RepositoryInfo' from line 10	reviewbot	Nov. 18, 2017, 8:59 p.m.
F811 redefinition of unused 'SCMClient' from line 10	reviewbot	Nov. 18, 2017, 8:59 p.m.
F811 redefinition of unused 'PatchResult' from line 10	reviewbot	Nov. 18, 2017, 8:59 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 18, 2017, 8:59 p.m.
F811 redefinition of unused 'PatchResult' from line 10	reviewbot	Nov. 18, 2017, 8:59 p.m.
F811 redefinition of unused 'RepositoryInfo' from line 10	reviewbot	Nov. 18, 2017, 8:59 p.m.
F811 redefinition of unused 'SCMClient' from line 10	reviewbot	Nov. 18, 2017, 8:59 p.m.
F401 'rbtools.hooks.common.get_review_request_id' imported but unused	reviewbot	Nov. 18, 2017, 8:59 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 18, 2017, 8:59 p.m.
E127 continuation line over-indented for visual indent	reviewbot	Nov. 18, 2017, 9:02 p.m.
E128 continuation line under-indented for visual indent	reviewbot	Nov. 18, 2017, 9:02 p.m.
W391 blank line at end of file	reviewbot	Nov. 18, 2017, 9:02 p.m.
E226 missing whitespace around arithmetic operator	reviewbot	Nov. 30, 2017, 3:36 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 25, 2017, 8:06 p.m.
Have you checked what happens when the commit message contains a colon+space separator (:)? The regex looks like it would …	bleblan2	Nov. 30, 2017, 3:42 p.m.
E226 missing whitespace around arithmetic operator	reviewbot	Nov. 30, 2017, 3:36 p.m.
E226 missing whitespace around arithmetic operator	reviewbot	Nov. 30, 2017, 3:37 p.m.
E127 continuation line over-indented for visual indent	reviewbot	Nov. 30, 2017, 3:42 p.m.
run_from_argv now returns the exit code, as opposed to calling sys.exit(exit_code). I'd consider adding what it returns to the function's …	NI nicho	Dec. 10, 2017, 12:37 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/commands/tests/test_repo_hook.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'json' imported but unused
```
rbtools/commands/tests/test_repo_hook.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'six.moves.urllib.request.urlopen' imported but unused
```
rbtools/commands/tests/test_repo_hook.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'rbtools.api.tests.MockResponse' imported but unused
```
rbtools/commands/tests/test_repo_hook.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'rbtools.clients.svn.SVNClient' imported but unused
```

Description:

~		Applies the 0001-WIP-Repository-hooks-support.patch that Christian Hammond gave me as a skeleton to start with. No other changes have been made yet, as I'm still going through the code.
	~	Applies the 0001-WIP-Repository-hooks-support.patch that Christian Hammond gave
	+	me as a skeleton to start with. No other changes have been made yet, as I'm
	+	still going through the code.
	+
	+	Oct 1 - 8
	+	Fixed common.py to make the old hook method work so I could test it and learn
	+	how it works. Found the common.py fix in https://reviews.reviewboard.org/r/9198/
	+	Started working through repo_hook.py and learned that the code Christian Hammond
	+	gave me comes with working git functionality.

Testing Done:

		Ran nosetests -v; all tests pass with patch applied.
	+
	+	Oct 1 - 8
	+	Manually tested the repo-hook command by making hooks for pre-receive and
	+	post-receive in a local remote repository. I made changes and pushed the changes
	+	to test the pre-receive hook, it denied the push as expected because the review
	+	request was not marked "Ship It". Allowing the push to succeed caused the
	+	post-receive code close the review-request.

Commit:

-	39f4e50ab9c9271572eab66b55b0cd2f2add6415
+	1ea1b5644830abc5a0f1a7caf72d5bf2e7c9333f

Diff:

Revision 2 (+409 -16)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/svn.py
	rbtools/commands/__init__.py
	rbtools/commands/main.py
	rbtools/commands/repo_hook.py
	rbtools/commands/tests/test_repo_hook.py
	3 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/tests/test_repo_hook.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E265 block comment should start with '# '
```
rbtools/commands/tests/test_repo_hook.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E265 block comment should start with '# '
```
rbtools/commands/tests/test_repo_hook.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E265 block comment should start with '# '
```
rbtools/commands/tests/test_repo_hook.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E265 block comment should start with '# '
```

Change Summary:

Working on SVN command

Commit:

-	1ea1b5644830abc5a0f1a7caf72d5bf2e7c9333f
+	bd3308235b7a6559c0dd3bea714f8c2c3ce0783b

Diff:

Revision 3 (+419 -16)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/svn.py
	rbtools/commands/__init__.py
	rbtools/commands/main.py
	rbtools/commands/repo_hook.py
	rbtools/commands/tests/test_repo_hook.py
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

SVN repo basic functionalty added. Messy but working, need to change to be consistent with Git.

Commit:

-	bd3308235b7a6559c0dd3bea714f8c2c3ce0783b
+	d76a380ab6e7560b4b7e6bb042cbe6aebc307c01

Diff:

Revision 4 (+445 -16)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/svn.py
	rbtools/commands/__init__.py
	rbtools/commands/main.py
	rbtools/commands/repo_hook.py
	rbtools/commands/tests/test_repo_hook.py
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Started svn repository hooks unit tests and added basics to Mercurial.

Commit:

-	d76a380ab6e7560b4b7e6bb042cbe6aebc307c01
+	c97ca4a0b39aff638a2ad0ab3f56befd41c5d37d

Diff:

Revision 5 (+466 -17)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py
	rbtools/commands/__init__.py
	rbtools/commands/main.py
	5 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/mercurial.py (Diff revision 5)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'RepositoryInfo' from line 10
```
rbtools/clients/mercurial.py (Diff revision 5)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'SCMClient' from line 10
```
rbtools/clients/mercurial.py (Diff revision 5)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'PatchResult' from line 10
```
rbtools/clients/mercurial.py (Diff revision 5)
The issue has been resolved. Show all issues
```
E302 expected 2 blank lines, found 1
```

Commit:

-	c97ca4a0b39aff638a2ad0ab3f56befd41c5d37d
+	ba97b9f4c0ba14ed0b9559eefdf10072c8a3c9ed

Diff:

Revision 6 (+501 -17)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py
	rbtools/commands/__init__.py
	rbtools/commands/main.py
	5 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/mercurial.py (Diff revision 6)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'PatchResult' from line 10
```
rbtools/clients/mercurial.py (Diff revision 6)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'RepositoryInfo' from line 10
```
rbtools/clients/mercurial.py (Diff revision 6)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'SCMClient' from line 10
```
rbtools/clients/mercurial.py (Diff revision 6)
The issue has been resolved. Show all issues
```
F401 'rbtools.hooks.common.get_review_request_id' imported but unused
```
rbtools/clients/mercurial.py (Diff revision 6)
The issue has been resolved. Show all issues
```
E302 expected 2 blank lines, found 1
```
rbtools/clients/tests/test_svn.py (Diff revision 6)
The issue has been resolved. Show all issues
```
E127 continuation line over-indented for visual indent
```
rbtools/clients/tests/test_svn.py (Diff revision 6)
The issue has been resolved. Show all issues
```
E128 continuation line under-indented for visual indent
```
rbtools/clients/tests/test_svn.py (Diff revision 6)
The issue has been resolved. Show all issues
```
W391 blank line at end of file
```

Change Summary:

Working Mercurial pre-commit hooks. Fixed Flake8 errors.

Commit:

-	ba97b9f4c0ba14ed0b9559eefdf10072c8a3c9ed
+	1a4d2395433934d6f0ac781bc96a4e406c8c71a3

Diff:

Revision 7 (+519 -18)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py
	rbtools/commands/__init__.py
	rbtools/commands/main.py
	5 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/mercurial.py (Diff revision 7)
The issue has been resolved. Show all issues
```
E226 missing whitespace around arithmetic operator
```
rbtools/commands/repo_hook.py (Diff revision 7)
The issue has been resolved. Show all issues
```
E303 too many blank lines (2)
```

Change Summary:

Added Mercurial tests, Changed SVN to make Post-commit work, still not working atm. Changed Repo_hook to use command arguments for input

Commit:

-	1a4d2395433934d6f0ac781bc96a4e406c8c71a3
+	769cf363965c7c286c9da5893d9b53bf807fa23b

Diff:

Revision 8 (+559 -19)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_mercurial.py
	rbtools/clients/tests/test_svn.py
	rbtools/commands/__init__.py
	6 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/mercurial.py (Diff revision 8)
The issue has been dropped. Show all issues
```
E226 missing whitespace around arithmetic operator
```
rbtools/clients/tests/test_mercurial.py (Diff revision 8)
The issue has been dropped. Show all issues
```
E226 missing whitespace around arithmetic operator
```
rbtools/commands/repo_hook.py (Diff revision 8)
The issue has been resolved. Show all issues
```
E127 continuation line over-indented for visual indent
```

Just a note about the regex being used for parsing the mercurial logs.

rbtools/clients/mercurial.py (Diff revision 8)

The issue has been resolved. Show all issues

Have you checked what happens when the commit message contains a colon+space separator (:)? The regex looks like it would end up splitting a commit message if it is there.

I'm also guessing the regex would split multiline commit messages at the newline.

GrahamS Nov. 30, 2017, 1:10 p.m.

Good call. It will mess up. Thanks, I'll change the way I do it.

Change Summary:

Removed debuggin code. Fixed flake8 errors. Corrected Mercurial based on feedback

Commit:

-	769cf363965c7c286c9da5893d9b53bf807fa23b
+	f8879b38170101b908ddff0cf96adb46c87398f1

Diff:

Revision 9 (+557 -19)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_mercurial.py
	rbtools/clients/tests/test_svn.py
	rbtools/commands/__init__.py
	6 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Added docstrings to functions and classes. Should be good, just need to finish the RST doc.

Summary:

-	[WIP] Making it easy to write pre/post-commit hooks
+	Making it easy to write pre/post-commit hooks

Description:

~		Applies the 0001-WIP-Repository-hooks-support.patch that Christian Hammond gave
~		me as a skeleton to start with. No other changes have been made yet, as I'm
	~	Applies the 0001-WIP-Repository-hooks-support.patch that Christian Hammond
	~	created.
-		still going through the code.

~		Oct 1 - 8
~		Fixed common.py to make the old hook method work so I could test it and learn
	~	Adds the repo-hook command to make it easy to use pre and post commit hooks. The
	~	supported repositories are Git, Mercurial and SVN.
-		how it works. Found the common.py fix in https://reviews.reviewboard.org/r/9198/
-		Started working through repo_hook.py and learned that the code Christian Hammond
-		gave me comes with working git functionality.

Testing Done:

		Ran nosetests -v; all tests pass with patch applied.

~		Oct 1 - 8
~		Manually tested the repo-hook command by making hooks for pre-receive and
~		post-receive in a local remote repository. I made changes and pushed the changes
	~	Created unit tests for all functions.
	~
	~	Ran nosetests -v; all tests passed.
-		to test the pre-receive hook, it denied the push as expected because the review
-		request was not marked "Ship It". Allowing the push to succeed caused the
-		post-receive code close the review-request.

Commit:

-	f8879b38170101b908ddff0cf96adb46c87398f1
+	5389c2a7fde0d8bd870d43aecad9e328fcc63c94

Diff:

Revision 10 (+717 -19)

Show changes

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_mercurial.py
	rbtools/clients/tests/test_svn.py
	rbtools/commands/__init__.py
	6 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Added repo-hook.rst documentation and forced api-token for repo-hook command to work

Commit:

-	5389c2a7fde0d8bd870d43aecad9e328fcc63c94
+	caf11f32eeda8b866c7b0dde79d1982030178579

Diff:

Revision 11 (+795 -19)

Show changes

	setup.py
	docs/rbtools/rbt/commands/repo-hook.rst
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_mercurial.py
	rbtools/clients/tests/test_svn.py
	7 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/commands/__init__.py (Diff revision 11)

The issue has been resolved. Show all issues

run_from_argv now returns the exit code, as opposed to calling sys.exit(exit_code). I'd consider adding what it returns to the function's docstring.

Change Summary:

Added return value to the docstring for run_from_argv function

Commit:

-	caf11f32eeda8b866c7b0dde79d1982030178579
+	896d777e4482f97d8b17bf0d6ec4568f69dcdc69

Diff:

Revision 12 (+800 -19)

Show changes

	setup.py
	docs/rbtools/rbt/commands/repo-hook.rst
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_mercurial.py
	rbtools/clients/tests/test_svn.py
	7 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Fixed spelling and added Git docstrings.

Commit:

-	896d777e4482f97d8b17bf0d6ec4568f69dcdc69
+	ec738867fe27c1d9a6c915954a5bc236190fb8ce

Diff:

Revision 13 (+816 -19)

Show changes

	setup.py
	docs/rbtools/rbt/commands/repo-hook.rst
	rbtools/clients/__init__.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/svn.py
	rbtools/clients/tests/test_mercurial.py
	rbtools/clients/tests/test_svn.py
	7 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Making it easy to write pre/post-commit hooks

Information

Reviewers

flake8

Description:

Testing Done:

Commit:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Commit:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Change Summary:

Commit:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Change Summary:

Commit:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Summary:

Description:

Testing Done:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Status: Discarded