-
-
rbtools/hooks/common.py (Diff revision 1) There should be two blank lines between items at the lowest indentation level. Here and below.
-
rbtools/hooks/common.py (Diff revision 1) This should probably propagate up the message in the exception.
-
rbtools/hooks/common.py (Diff revision 1) This should probably check that the command was sucessful.
-
rbtools/hooks/common.py (Diff revision 1) Should it be up to the caller to pass in a compiled regex with the options that they care to use?
-
rbtools/hooks/common.py (Diff revision 1) This can just return directly instead of assigning to a variable and returning that.
-
-
rbtools/hooks/git.py (Diff revision 1) It might be nice to pass in arguments as a list instead of relying on shell=True to split it.
-
rbtools/hooks/git.py (Diff revision 1) Relying on the pipe here makes it so this only works on unix-type systems (and even then relies a lot on shell=True parsing this). You could do this filtering pretty easily in python instead.
-
-
-
Add hooks module, containing utility functions common to hook scripts.
Review Request #5543 — Created Feb. 24, 2014 and submitted
This is a new 'hooks' module in RBTools. rbtools/hooks/common.py contains functions common to all hook scripts, and rbtools/hooks/git.py contains functions for Git hook scripts.
- Tested the functions individually, and got the expected results.
- More testing was done in https://reviews.reviewboard.org/r/5403/ for the Git post-receive and pre-receive hook scripts that use this new module.
Description | From | Last Updated |
---|---|---|
There should be two blank lines between items at the lowest indentation level. Here and below. |
|
|
This should probably propagate up the message in the exception. |
|
|
This should probably check that the command was sucessful. |
|
|
Should it be up to the caller to pass in a compiled regex with the options that they care to … |
|
|
This can just return directly instead of assigning to a variable and returning that. |
|
|
Two blank lines between top-level blocks. Here and below. |
|
|
It might be nice to pass in arguments as a list instead of relying on shell=True to split it. |
|
|
Relying on the pipe here makes it so this only works on unix-type systems (and even then relies a lot … |
|
|
You could use '0' * 40 here. |
|
|
No need for this line. |
|
|
No need for this line. |
|
|
"URL" |
|
|
So I believe that Christian's comment about _make_api_client was that it's only used in this one place, so instead of … |
|
Change Summary:
Fixed some style issues, and modified execute statements so we no longer rely on shell=True.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+214) |
-
Guess I never published this. Meant to :/
-
rbtools/hooks/common.py (Diff revision 2) Just curious. Why add this function instead of just instantiating RBClient where needed?
-
Change Summary:
Fixed capitalization of 'URL'.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+214) |
Change Summary:
Fixed a docstring.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+214) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+216) |
-
-
rbtools/hooks/common.py (Diff revision 5) So I believe that Christian's comment about
_make_api_client
was that it's only used in this one place, so instead of wrapping it in a function you can just replace this line with:api_client = RBClient(server_url, username=username, password=password)
Change Summary:
Changed all review request ID variables to ints, and added a log formatter.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+216) |
Change Summary:
Fixed a bug that caused the pre-receive hook to crash for a push with a newly created branch. (We need to run git rev-list using new_rev, not ref_name. The branch ref_name will not yet exist before the push, which is when the pre-receive hook is executed.)
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+216) |