git cl try: Remove --name option, stop sending "reason" property.

git_cl.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld and Gerrit."""
 
 from __future__ import print_function
@@ skipping 372 matching lines @@
           'changes': [{
               'author': {'email': owner_email},
               'revision': options.revision,
           }],
           'properties': {
               'category': category,
               'issue': changelist.GetIssue(),
               'patch_project': project,
               'patch_storage': 'rietveld',
               'patchset': patchset,
-              'reason': options.name,
+              'reason': options.reason,
               'rietveld': codereview_url,
           },
       }
       if 'presubmit' in builder.lower():
         parameters['properties']['dry_run'] = 'true'
       if tests:
         parameters['properties']['testfilter'] = tests
       if extra_properties:
         parameters['properties'].update(extra_properties)
       if options.clobber:
@@ skipping 4357 matching lines @@
       help='Override which project to use. Projects are defined '
            'in recipe to determine to which repository or directory to '
            'apply the patch')
   group.add_option(
       '-p', '--property', dest='properties', action='append', default=[],
       help='Specify generic properties in the form -p key1=value1 -p '
            'key2=value2 etc. The value will be treated as '
            'json if decodable, or as string otherwise. '
            'NOTE: using this may make your try job not usable for CQ, '
            'which will then schedule another try job with default properties')
-  # TODO(tandrii): if this even used?

[qyearsley, 2016/10/22 19:14:29]

It's used for the "reason" string when triggering a build, but I'm not sure if
*that* is used or necessary. Do you know?

[tandrii(chromium), 2016/10/23 15:25:52]

not necessary, ignored by CQ (though CQ sets it to 'cq')

   group.add_option(
-      '-n', '--name', help='Try job name; default to current branch name')
+      '--reason',
+      help='Try job "reason" string; defaults to current branch name')

[tandrii(chromium), 2016/10/23 15:25:52]

i like this renaming, but please you remove default value completely (ie
default=None)
I'd also not specify in list of tryjob properties at all if it's None.

[qyearsley, 2016/10/25 17:32:13]

Do you mean let's only add it to try job properties if it's specified?

Actually, would you be OK with removing it completely and never adding "reason"
to try job properties?

[tandrii(chromium), 2016/10/25 17:56:30]

yes, PS#2 is exactly what I asked for :)
Hm, come to think about this - we have -p|--property flag, so reason can still
be specified if necessary. So, hell yeah. But if you do that, please send PSA to
infra-announce@chromium.dev and chrome-infrastructure-announce@ MLs with link to
this CL.

And in any case, please update description of this CL.

   group.add_option(
       '--buildbucket-host', default='cr-buildbucket.appspot.com',
       help='Host of buildbucket. The default host is %default.')
   parser.add_option_group(group)
   auth.add_auth_options(parser)
   options, args = parser.parse_args(args)
   auth_config = auth.extract_auth_config_from_options(options)
 
   # Make sure that all properties are prop=value pairs.
   bad_params = [x for x in options.properties if '=' not in x]
@@ skipping 10 matching lines @@
   if cl.IsGerrit():
     parser.error(
         'Not yet supported for Gerrit (http://crbug.com/599931).\n'
         'If your project has Commit Queue, dry run is a workaround:\n'
         '  git cl set-commit --dry-run')
 
   error_message = cl.CannotTriggerTryJobReason()
   if error_message:
     parser.error('Can\'t trigger try jobs: %s')
 
-  if not options.name:
-    options.name = cl.GetBranch()
+  if not options.reason:

[tandrii(chromium), 2016/10/23 15:25:52]

that is, let's get rid of these two lines.

+    options.reason = cl.GetBranch()
 
   if options.bucket and options.master:
     parser.error('Only one of --bucket and --master may be used.')
 
   if options.bot and not options.master and not options.bucket:
     options.master, err_msg = GetBuilderMaster(options.bot)
     if err_msg:
       parser.error('Tryserver master cannot be found because: %s\n'
                    'Please manually specify the tryserver master'
                    ', e.g. "-m tryserver.chromium.linux".' % err_msg)
@@ skipping 598 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   setup_color.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)