From a089281a82f9481e246dcc1292145d0e8635f51c Mon Sep 17 00:00:00 2001
From: Thiago Perrotta <tperrotta@chromium.org>
Date: Sat, 3 Sep 2022 11:22:46 +0000
Subject: [PATCH] roll_dep: support multiple -r/--reviewer arguments

Currently multiple reviewers can only be provided with a comma-separated
list, for example: -r foo,bar,baz

This CL adds support for multiple -r arguments such as:

-r foo -r bar -r baz

Or even:

-r foo,bar -r baz

This makes it consistent (and thus less confusing) with `git cl upload` which does accept multiple -r arguments.

Bug: none
Change-Id: I27d03601b488c0c5b27568d86dcb4ed66df6b76f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3863675
Commit-Queue: Thiago Perrotta <tperrotta@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Auto-Submit: Thiago Perrotta <tperrotta@chromium.org>
---
 roll_dep.py            | 13 +++++++++----
 tests/roll_dep_test.py | 16 ++++++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/roll_dep.py b/roll_dep.py
index be8b6b09b4..3defd7048c 100755
--- a/roll_dep.py
+++ b/roll_dep.py
@@ -12,6 +12,7 @@ always roll to the tip of to origin/main.
 from __future__ import print_function
 
 import argparse
+import itertools
 import os
 import re
 import subprocess2
@@ -200,9 +201,13 @@ def main():
       '--ignore-dirty-tree', action='store_true',
       help='Roll anyways, even if there is a diff.')
   parser.add_argument(
-      '-r', '--reviewer',
-      help='To specify multiple reviewers, use comma separated list, e.g. '
-           '-r joe,jane,john. Defaults to @chromium.org')
+      '-r',
+      '--reviewer',
+      action='append',
+      help=
+      'To specify multiple reviewers, either use a comma separated list, e.g. '
+      '-r joe,jane,john or provide the flag multiple times, e.g. '
+      '-r joe -r jane. Defaults to @chromium.org')
   parser.add_argument('-b', '--bug', help='Associate a bug number to the roll')
   # It is important that --no-log continues to work, as it is used by
   # internal -> external rollers. Please do not remove or break it.
@@ -230,7 +235,7 @@ def main():
           'Can\'t use multiple paths to roll simultaneously and --key')
   reviewers = None
   if args.reviewer:
-    reviewers = args.reviewer.split(',')
+    reviewers = list(itertools.chain(*[r.split(',') for r in args.reviewer]))
     for i, r in enumerate(reviewers):
       if not '@' in r:
         reviewers[i] = r + '@chromium.org'
diff --git a/tests/roll_dep_test.py b/tests/roll_dep_test.py
index 514d5d9950..7ce0cebbe9 100755
--- a/tests/roll_dep_test.py
+++ b/tests/roll_dep_test.py
@@ -114,6 +114,22 @@ class RollDepTest(fake_repos.FakeReposTestBase):
     self.assertIn(expected_message, stdout)
     self.assertIn(expected_message, commit_message)
 
+  def testRollsDepReviewers(self):
+    if not self.enabled:
+      return
+
+    stdout, stderr, returncode = self.call([
+        ROLL_DEP, 'src/foo', '-r', 'foo@example.com', '-r',
+        'bar@example.com,baz@example.com'
+    ])
+
+    self.assertEqual(stderr, '')
+    self.assertEqual(returncode, 0)
+
+    expected_message = 'R=foo@example.com,bar@example.com,baz@example.com'
+
+    self.assertIn(expected_message, stdout)
+
   def testRollsDepToSpecificRevision(self):
     if not self.enabled:
       return