From c4ac30261d1a8383ff6f8acfca85fa21ce50e85b Mon Sep 17 00:00:00 2001 From: Joanna Wang Date: Tue, 31 Jan 2023 21:19:57 +0000 Subject: [PATCH] Revert auto-cc behavior back to original and fix comments. Bug: 1411521, b/265929888 Change-Id: I2d5908f4fb54ad11914d061539c4677e8320c163 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4205471 Reviewed-by: Gavin Mak Commit-Queue: Joanna Wang --- git_cl.py | 15 ++++++--------- tests/git_cl_test.py | 14 +++++++------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/git_cl.py b/git_cl.py index 5d37c6c83..e827a33f3 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1793,9 +1793,9 @@ class Changelist(object): # Add ccs ccs = [] - # Add default, watchlist, presubmit ccs if this is an existing change + # Add default, watchlist, presubmit ccs if this is the initial upload # and CL is not private and auto-ccing has not been disabled. - if self.GetIssue() and not (options.private and options.no_autocc): + if not options.private and not options.no_autocc and not self.GetIssue(): ccs = self.GetCCList().split(',') if len(ccs) > 100: lsc = ('https://chromium.googlesource.com/chromium/src/+/HEAD/docs/' @@ -2743,7 +2743,7 @@ class Changelist(object): change_desc, branch): """Upload the current branch to Gerrit.""" if options.squash: - self._GerritCommitMsgHookCheck(offer_removal=not options.force) + Changelist._GerritCommitMsgHookCheck(offer_removal=not options.force) external_parent = None if self.GetIssue(): # User requested to change description @@ -2815,9 +2815,9 @@ class Changelist(object): reviewers = sorted(change_desc.get_reviewers()) cc = [] - # Add CCs from WATCHLISTS and rietveld.cc git config unless this is - # the initial upload, the CL is private, or auto-CCing has ben disabled. - if not (self.GetIssue() or options.private or options.no_autocc): + # Add default, watchlist, presubmit ccs if this is the initial upload + # and CL is not private and auto-ccing has not been disabled. + if not options.private and not options.no_autocc and not self.GetIssue(): cc = self.GetCCList().split(',') if len(cc) > 100: lsc = ('https://chromium.googlesource.com/chromium/src/+/HEAD/docs/' @@ -4740,9 +4740,6 @@ def CMDupload(parser, args): parser.add_option('--no-python2-post-upload-hooks', action='store_true', help='Only run post-upload hooks in Python 3.') - parser.add_option('--stacked-exp', - action='store_true', - help=optparse.SUPPRESS_HELP) # TODO(b/265929888): Add --wip option of --cl-status option. orig_args = args diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index a72b40afc..f4e884e38 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -287,8 +287,8 @@ class TestGitClBasic(unittest.TestCase): return_value=('foo', git_cl.DEFAULT_NEW_BRANCH)).start() mock.patch('git_cl.GetTargetRef', return_value='refs/heads/main').start() - mock.patch('git_cl.Changelist._GerritCommitMsgHookCheck', - lambda _, offer_removal: None).start() + mock.patch('git_cl.Changelist._GerritCommitMsgHookCheck', lambda + offer_removal: None).start() mock.patch('git_cl.Changelist.GetIssue', return_value=None).start() mock.patch('git_cl.Changelist.GetBranch', side_effect=SystemExitMock).start() @@ -1126,8 +1126,8 @@ class TestGitCl(unittest.TestCase): mock.patch('git_cl.gerrit_util.CookiesAuthenticator', CookiesAuthenticatorMockFactory( same_auth=('git-owner.example.com', '', 'pass'))).start() - mock.patch('git_cl.Changelist._GerritCommitMsgHookCheck', - lambda _, offer_removal: None).start() + mock.patch('git_cl.Changelist._GerritCommitMsgHookCheck', lambda + offer_removal: None).start() mock.patch('git_cl.Changelist.GetMostRecentPatchset', lambda _, update: patchset).start() mock.patch('git_cl.gclient_utils.RunEditor', @@ -3846,6 +3846,7 @@ class ChangelistTest(unittest.TestCase): with self.assertRaises(SystemExitMock): cl.PrepareCherryPickSquashedCommit(options) + @mock.patch('git_cl.Settings.GetDefaultCCList', return_value=[]) @mock.patch('git_cl.Changelist.GetAffectedFiles', return_value=[]) @mock.patch('git_cl.GenerateGerritChangeId', return_value='1a2b3c') @mock.patch('git_cl.Changelist.GetIssue', return_value=None) @@ -3878,7 +3879,7 @@ class ChangelistTest(unittest.TestCase): reviewers, ccs, change_desc = cl._PrepareChange(options, parent, latest_tree) self.assertEqual(reviewers, ['horse@apple.farm']) - self.assertEqual(ccs, ['chicken@bok.farm', 'cow2@moo.farm']) + self.assertEqual(ccs, ['cow@moo.farm', 'chicken@bok.farm', 'cow2@moo.farm']) self.assertEqual(change_desc._description_lines, [ 'AH!', 'CC=cow2@moo.farm', 'R=horse@apple.farm', '', 'Change-Id: 1a2b3c' ]) @@ -3892,7 +3893,6 @@ class ChangelistTest(unittest.TestCase): description=desc, all_files=False) - @mock.patch('git_cl.Settings.GetDefaultCCList', return_value=[]) @mock.patch('git_cl.Changelist.GetAffectedFiles', return_value=[]) @mock.patch('git_cl.Changelist.GetIssue', return_value='123') @mock.patch('git_cl.ChangeDescription.prompt') @@ -3931,7 +3931,7 @@ class ChangelistTest(unittest.TestCase): reviewers, ccs, change_desc = cl._PrepareChange(options, parent, latest_tree) self.assertEqual(reviewers, ['horse@apple.farm']) - self.assertEqual(ccs, ['cow@moo.farm', 'chicken@bok.farm', 'cow2@moo.farm']) + self.assertEqual(ccs, ['chicken@bok.farm', 'cow2@moo.farm']) self.assertEqual(change_desc._description_lines, [ 'AH!', 'CC=cow2@moo.farm', 'R=horse@apple.farm', '', 'Change-Id: 123456789'