From bb7395009dd6209a97fc59ffce96c056b7133958 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Thu, 4 Dec 2014 11:42:00 -0500 Subject: [PATCH] Add reviewers on upload Add the --reviewers argument which allows specifying reviewers to be added to each patch uploaded. This simplifies the upload process by not requiring an additional step of adding reviewers to changes. Gerrit supports specifying reviewers to be added to changes by appending a list of r='email' options to the refspec being pushed to. For example, from the Gerrit 'Uploading Changes' documentation: git push tr:kernel/common HEAD:refs/for/experimental%r=a@a.com,cc=b@o.com The --reviewers argument can be passed multiple reviewers to add to the patch set. Reviewers containing whitespace are rejected, as whitespace cannot be added to the refspec. Change-Id: I8c2f9453a90dd78aa47f7996f2502f9f6cf2d66d --- doc/source/usage.rst | 4 ++++ git-review.1 | 5 +++++ git_review/cmd.py | 21 +++++++++++++++++++ git_review/tests/test_git_review.py | 32 +++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+) diff --git a/doc/source/usage.rst b/doc/source/usage.rst index a43c674..d430a4a 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -18,6 +18,10 @@ If you want to supply a review topic:: git review -t topic/awesome-feature +If you want to subscribe some reviewers:: + + git review --reviewers a@example.com b@example.com + If you want to disable autogenerated topic:: git review -T diff --git a/git-review.1 b/git-review.1 index 8515835..29c776f 100644 --- a/git-review.1 +++ b/git-review.1 @@ -41,6 +41,7 @@ .Op Fl fnuvDRT .Op Fl r Ar remote .Op Fl t Ar topic +.Op Fl \-reviewers Ar reviewer ... .Op Ar branch .Nm .Fl \-version @@ -140,6 +141,8 @@ Sets the target topic for this change on the gerrit server. If not specified, a bug number from the commit summary will be used. Alternatively, the local branch name will be used if different from remote branch. .It Fl T , Fl \-no\-topic Submit review without topic. +.It Fl \-reviewers Ar reviewer ... +Subscribe one or more reviewers to the uploaded patch sets. Reviewers should be identifiable by Gerrit (usually use their Gerrit username or email address). .It Fl u , Fl \-update Skip cached local copies and force updates from network resources. .It Fl l , Fl \-list @@ -337,6 +340,8 @@ specific error codes are available: Gerrit .Ar commit\-msg hook could not be successfully installed. +.It 3 +Could not parse malformed argument value or user input. .It 32 Cannot fetch list of open changesets from Gerrit. .It 33 diff --git a/git_review/cmd.py b/git_review/cmd.py index 1989971..296341b 100755 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -1217,6 +1217,20 @@ def convert_bool(one_or_zero): return str(one_or_zero) in ["1", "true", "True"] +class MalformedInput(GitReviewException): + EXIT_CODE = 3 + + +def assert_valid_reviewers(reviewers): + """Ensure no whitespace is found in reviewer names, as it will result + in an invalid refspec. + """ + for reviewer in reviewers: + if re.search(r'\s', reviewer): + raise MalformedInput( + "Whitespace not allowed in reviewer: '%s'" % reviewer) + + def _main(): usage = "git review [OPTIONS] ... [BRANCH]" @@ -1237,6 +1251,8 @@ def _main(): action="store_true", help="No topic except if explicitly provided") + parser.add_argument("--reviewers", nargs="+", + help="Add reviewers to uploaded patch sets.") parser.add_argument("-D", "--draft", dest="draft", action="store_true", help="Submit review as a draft") parser.add_argument("-c", "--compatible", dest="compatible", @@ -1448,6 +1464,11 @@ def _main(): topic = None if options.notopic else get_topic(branch) if topic and topic != branch: cmd += "/%s" % topic + + if options.reviewers: + assert_valid_reviewers(options.reviewers) + cmd += "%" + ",".join("r=%s" % r for r in options.reviewers) + if options.regenerate: print("Amending the commit to regenerate the change id\n") regenerate_cmd = "git commit --amend" diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py index 680c08d..27adc98 100644 --- a/git_review/tests/test_git_review.py +++ b/git_review/tests/test_git_review.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import os import shutil @@ -128,6 +129,37 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): review_res = self._run_git_review('-y') self.assertIn("Processing changes: new: 2", review_res) + def test_git_review_re(self): + """Test git-review adding reviewers to changes.""" + self._run_git_review('-s') + + # Create users to add as reviewers + self._run_gerrit_cli('create-account', '--email', + 'reviewer1@example.com', 'reviewer1') + self._run_gerrit_cli('create-account', '--email', + 'reviewer2@example.com', 'reviewer2') + + self._simple_change('test file', 'test commit message') + + review_res = self._run_git_review('--reviewers', 'reviewer1', + 'reviewer2') + self.assertIn("Processing changes: new: 1", review_res) + + # verify both reviewers are on patch set + head = self._run_git('rev-parse', 'HEAD') + change = self._run_gerrit_cli('query', '--format=JSON', + '--all-reviewers', head) + # The first result should be the one we want + change = json.loads(change.split('\n')[0]) + + self.assertEqual(2, len(change['allReviewers'])) + + reviewers = set() + for reviewer in change['allReviewers']: + reviewers.add(reviewer['username']) + + self.assertEqual(set(['reviewer1', 'reviewer2']), reviewers) + def test_rebase_no_remote_branch_msg(self): """Test message displayed where no remote branch exists.""" self._run_git_review('-s')