Reland "Remove unnecessary notify=None from git cl upload."

This reverts commit cea35a3f771ad3c22a35be25b88aa3163a9457cc.

Reason for revert: b/298657697 is unrelated to this

Original change's description:
> Revert "Remove unnecessary notify=None from git cl upload."
>
> This reverts commit 7688e784503525b598a18991e190038381c333cf.
>
> Reason for revert: afaict there is a gerrit notifications bug b/297928626 unrelated to this change but i'm reverting this anyway and relanding when the gerrit bug is resolved.
>
> Original change's description:
> > Remove unnecessary notify=None from git cl upload.
> >
> > This is a NOOP change for all cases except for "Publish comments on push" which gets fixed with this CL.
> > Our hosts have the notify on each patchset turned off.
> >
> > Bug: 1472724
> > Change-Id: I3672c383b1e4ca1f6243c9b9d2c906473f5037d9
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4797981
> > Reviewed-by: Gavin Mak <gavinmak@google.com>
> > Commit-Queue: Joanna Wang <jojwang@chromium.org>
>
> Bug: 1472724
> Change-Id: I3de1a25fe84a21a2adb8b4bbddb460dd45530418
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4817767
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Joanna Wang <jojwang@chromium.org>

Bug: 1472724
Change-Id: I63cdb99111ae89eb6edd5172d827f84b314c015e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4852217
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>

git_cl.py

@@ -1870,8 +1870,6 @@ class Changelist(object):
             refspec_opts.append('notify=ALL')
         elif (not self.GetIssue() and options.squash and not dogfood_path):
             refspec_opts.append('wip')
-        else:
-            refspec_opts.append('notify=NONE')
 
         # TODO(tandrii): options.message should be posted as a comment if
         # --send-mail or --send-email is set on non-initial upload as Rietveld

tests/git_cl_test.py

@@ -904,17 +904,13 @@ class TestGitCl(unittest.TestCase):
         ]
 
         metrics_arguments = []
-
+        ref_suffix_list = []
         if notify:
-            ref_suffix = '%ready,notify=ALL'
+            ref_suffix_list.append('ready,notify=ALL')
             metrics_arguments += ['ready', 'notify=ALL']
-        else:
-            if not issue and squash:
-                ref_suffix = '%wip'
-                metrics_arguments.append('wip')
-            else:
-                ref_suffix = '%notify=NONE'
-                metrics_arguments.append('notify=NONE')
+        elif not issue and squash:
+            ref_suffix_list.append('wip')
+            metrics_arguments.append('wip')
 
         # If issue is given, then description is fetched from Gerrit instead.
         if not title:
@@ -928,22 +924,23 @@ class TestGitCl(unittest.TestCase):
                 ]
                 title = 'User input'
         if title:
-            ref_suffix += ',m=' + gerrit_util.PercentEncodeForGitRef(title)
+            ref_suffix_list.append('m=' +
+                                   gerrit_util.PercentEncodeForGitRef(title))
             metrics_arguments.append('m')
 
         for k, v in sorted((labels or {}).items()):
-            ref_suffix += ',l=%s+%d' % (k, v)
+            ref_suffix_list.append('l=%s+%d' % (k, v))
             metrics_arguments.append('l=%s+%d' % (k, v))
 
         if short_hostname == 'chromium':
             # All reviewers and ccs get into ref_suffix.
             for r in sorted(reviewers):
-                ref_suffix += ',r=%s' % r
+                ref_suffix_list.append('r=%s' % r)
                 metrics_arguments.append('r')
             if issue is None:
                 cc += ['test-more-cc@chromium.org', 'joe@example.com']
             for c in sorted(cc):
-                ref_suffix += ',cc=%s' % c
+                ref_suffix_list.append('cc=%s' % c)
                 metrics_arguments.append('cc')
             reviewers, cc = [], []
         else:
@@ -960,17 +957,20 @@ class TestGitCl(unittest.TestCase):
                        })]
             for r in sorted(reviewers):
                 if r != 'bad-account-or-email':
-                    ref_suffix += ',r=%s' % r
+                    ref_suffix_list.append('r=%s' % r)
                     metrics_arguments.append('r')
                     reviewers.remove(r)
             if issue is None:
                 cc += ['joe@example.com']
             for c in sorted(cc):
-                ref_suffix += ',cc=%s' % c
+                ref_suffix_list.append('cc=%s' % c)
                 metrics_arguments.append('cc')
                 if c in cc:
                     cc.remove(c)
 
+        ref_suffix = ''
+        if ref_suffix_list:
+            ref_suffix = '%' + ','.join(ref_suffix_list)
         calls += [
             (
                 ('time.time', ),
@@ -1586,12 +1586,9 @@ class TestGitCl(unittest.TestCase):
         # Asserts
         mockCherryPickCommit.assert_called_once_with(options,
                                                      upstream_gerrit_commit)
-        expected_refspec = (
-            'commit-to-push:refs/for/refs/heads/main%notify=NONE,'
-            'm=honk_stonk,topic=circus,hashtag=cow')
-        expected_refspec_opts = [
-            'notify=NONE', 'm=honk_stonk', 'topic=circus', 'hashtag=cow'
-        ]
+        expected_refspec = ('commit-to-push:refs/for/refs/heads/main%'
+                            'm=honk_stonk,topic=circus,hashtag=cow')
+        expected_refspec_opts = ['m=honk_stonk', 'topic=circus', 'hashtag=cow']
         mockRunGitPush.assert_called_once_with(expected_refspec,
                                                expected_refspec_opts, mock.ANY,
                                                options.push_options)
@@ -1685,10 +1682,9 @@ class TestGitCl(unittest.TestCase):
                       end_commit=None)
         ])
 
-        expected_refspec = (
-            'commit-to-push:refs/for/refs/heads/main%notify=NONE,'
-            'topic=circus,hashtag=cow')
-        expected_refspec_opts = ['notify=NONE', 'topic=circus', 'hashtag=cow']
+        expected_refspec = ('commit-to-push:refs/for/refs/heads/main%'
+                            'topic=circus,hashtag=cow')
+        expected_refspec_opts = ['topic=circus', 'hashtag=cow']
         mockRunGitPush.assert_called_once_with(expected_refspec,
                                                expected_refspec_opts, mock.ANY,
                                                options.push_options)
@@ -1770,12 +1766,9 @@ class TestGitCl(unittest.TestCase):
                 options, 'external-commit', 'external-commit', end_commit=None)
         ])
 
-        expected_refspec = (
-            'commit-to-push:refs/for/refs/heads/main%notify=NONE,'
-            'm=honk_stonk,topic=circus,hashtag=cow')
-        expected_refspec_opts = [
-            'notify=NONE', 'm=honk_stonk', 'topic=circus', 'hashtag=cow'
-        ]
+        expected_refspec = ('commit-to-push:refs/for/refs/heads/main%'
+                            'm=honk_stonk,topic=circus,hashtag=cow')
+        expected_refspec_opts = ['m=honk_stonk', 'topic=circus', 'hashtag=cow']
         mockRunGitPush.assert_called_once_with(expected_refspec,
                                                expected_refspec_opts, mock.ANY,
                                                options.push_options)