Added hyphen-only options

Added hyphen-only options

Some options have words separated by underscores. Added options with
same name and underscores replaced by hyphens.

BUG=400953
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=288366

[pgervais, 2014-08-01 23:29:55]

Just to reduce mental load a little bit.

[iannucci, 2014-08-01 23:33:40]

On 2014/08/01 23:29:55, pgervais wrote:
> Just to reduce mental load a little bit.

I'm ok with this (in general, across all infra tools), as long as we also keep
the underscore version too. My biggest gripe with the dash flavor is that it
doesn't show up in the code when you grep for it (since argparse magically
underscores it.

I would say, though, if you're going to do this, do it to all flags which have
an underscore presently (at least in the files you're touching).

[pgervais, 2014-08-01 23:47:32]

On 2014/08/01 23:33:40, iannucci wrote:
> On 2014/08/01 23:29:55, pgervais wrote:
> > Just to reduce mental load a little bit.
> 
> I'm ok with this (in general, across all infra tools), as long as we also keep
> the underscore version too. My biggest gripe with the dash flavor is that it
> doesn't show up in the code when you grep for it (since argparse magically
> underscores it.
> 
> I would say, though, if you're going to do this, do it to all flags which have
> an underscore presently (at least in the files you're touching).

That's what I intended. Did I forget some of them?

[iannucci, 2014-08-01 23:49:21]

https://codereview.chromium.org/436963005/diff/1/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/436963005/diff/1/git_cache.py#newcode545
git_cache.py:545: parser.add_option('--no_bootstrap', action='store_true',
this one?

https://codereview.chromium.org/436963005/diff/1/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1719
git_cl.py:1719: parser.add_option('-s', '--send-mail', action='store_true',
this should gain --send_mail

https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1726
git_cl.py:1726: parser.add_option('-c', '--use-commit-queue',
action='store_true',
same

https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1736
git_cl.py:1736: parser.add_option('--auto-bots', default=False,
action='store_true',
same

https://codereview.chromium.org/436963005/diff/1/git_new_branch.py
File git_new_branch.py (right):

https://codereview.chromium.org/436963005/diff/1/git_new_branch.py#newcode21
git_new_branch.py:21: g.add_argument('--upstream-current', action='store_true',
replaced, not added

[pgervais, 2014-08-02 00:25:01]

On 2014/08/01 23:49:21, iannucci wrote:
> https://codereview.chromium.org/436963005/diff/1/git_cache.py
> File git_cache.py (right):
> 
> https://codereview.chromium.org/436963005/diff/1/git_cache.py#newcode545
> git_cache.py:545: parser.add_option('--no_bootstrap', action='store_true',
> this one?
> 
> https://codereview.chromium.org/436963005/diff/1/git_cl.py
> File git_cl.py (right):
> 
> https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1719
> git_cl.py:1719: parser.add_option('-s', '--send-mail', action='store_true',
> this should gain --send_mail
> 
> https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1726
> git_cl.py:1726: parser.add_option('-c', '--use-commit-queue',
> action='store_true',
> same
> 
> https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1736
> git_cl.py:1736: parser.add_option('--auto-bots', default=False,
> action='store_true',
> same
> 
> https://codereview.chromium.org/436963005/diff/1/git_new_branch.py
> File git_new_branch.py (right):
> 
> https://codereview.chromium.org/436963005/diff/1/git_new_branch.py#newcode21
> git_new_branch.py:21: g.add_argument('--upstream-current',
action='store_true',
> replaced, not added

I replaced it because I believe we should not use underscores in option names.
I do not really buy your argument about grepping, because it only makes sense
inside the python file.
Having two long options which differ only by underscore / hyphen will lead to
some confusion in calling code. We'll end up with half of the calling code using
one form, and the other half using the other one. So we'll have to grep for both
forms anyway.

New patch on its way.

[pgervais, 2014-08-02 00:27:52]

On 2014/08/02 00:25:01, pgervais wrote:
> On 2014/08/01 23:49:21, iannucci wrote:
> > https://codereview.chromium.org/436963005/diff/1/git_cache.py
> > File git_cache.py (right):
> > 
> > https://codereview.chromium.org/436963005/diff/1/git_cache.py#newcode545
> > git_cache.py:545: parser.add_option('--no_bootstrap', action='store_true',
> > this one?
> > 
> > https://codereview.chromium.org/436963005/diff/1/git_cl.py
> > File git_cl.py (right):
> > 
> > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1719
> > git_cl.py:1719: parser.add_option('-s', '--send-mail', action='store_true',
> > this should gain --send_mail
> > 
> > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1726
> > git_cl.py:1726: parser.add_option('-c', '--use-commit-queue',
> > action='store_true',
> > same
> > 
> > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1736
> > git_cl.py:1736: parser.add_option('--auto-bots', default=False,
> > action='store_true',
> > same
> > 
> > https://codereview.chromium.org/436963005/diff/1/git_new_branch.py
> > File git_new_branch.py (right):
> > 
> > https://codereview.chromium.org/436963005/diff/1/git_new_branch.py#newcode21
> > git_new_branch.py:21: g.add_argument('--upstream-current',
> action='store_true',
> > replaced, not added
> 
> I replaced it because I believe we should not use underscores in option names.
> I do not really buy your argument about grepping, because it only makes sense
> inside the python file.
> Having two long options which differ only by underscore / hyphen will lead to
> some confusion in calling code. We'll end up with half of the calling code
using
> one form, and the other half using the other one. So we'll have to grep for
both
> forms anyway.
> 
> New patch on its way.

To be more precise: I *added* hyphenated options instead of replacing them
because I don't want to break existing calling code. But I'd like to clean that
up in the future. The only option I renamed I was pretty sure was not used in
slaves (tell me if I'm wrong).

[iannucci, 2014-08-02 00:30:15]

On 2014/08/02 00:27:52, pgervais wrote:
> On 2014/08/02 00:25:01, pgervais wrote:
> > On 2014/08/01 23:49:21, iannucci wrote:
> > > https://codereview.chromium.org/436963005/diff/1/git_cache.py
> > > File git_cache.py (right):
> > > 
> > > https://codereview.chromium.org/436963005/diff/1/git_cache.py#newcode545
> > > git_cache.py:545: parser.add_option('--no_bootstrap', action='store_true',
> > > this one?
> > > 
> > > https://codereview.chromium.org/436963005/diff/1/git_cl.py
> > > File git_cl.py (right):
> > > 
> > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1719
> > > git_cl.py:1719: parser.add_option('-s', '--send-mail',
action='store_true',
> > > this should gain --send_mail
> > > 
> > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1726
> > > git_cl.py:1726: parser.add_option('-c', '--use-commit-queue',
> > > action='store_true',
> > > same
> > > 
> > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1736
> > > git_cl.py:1736: parser.add_option('--auto-bots', default=False,
> > > action='store_true',
> > > same
> > > 
> > > https://codereview.chromium.org/436963005/diff/1/git_new_branch.py
> > > File git_new_branch.py (right):
> > > 
> > >
https://codereview.chromium.org/436963005/diff/1/git_new_branch.py#newcode21
> > > git_new_branch.py:21: g.add_argument('--upstream-current',
> > action='store_true',
> > > replaced, not added
> > 
> > I replaced it because I believe we should not use underscores in option
names.
> > I do not really buy your argument about grepping, because it only makes
sense
> > inside the python file.
> > Having two long options which differ only by underscore / hyphen will lead
to
> > some confusion in calling code. We'll end up with half of the calling code
> using
> > one form, and the other half using the other one. So we'll have to grep for
> both
> > forms anyway.
> > 
> > New patch on its way.
> 
> To be more precise: I *added* hyphenated options instead of replacing them
> because I don't want to break existing calling code. But I'd like to clean
that
> up in the future. The only option I renamed I was pretty sure was not used in
> slaves (tell me if I'm wrong).

It's either used by slaves, and it will break, or it's used by devs, and they
will probably be annoyed.

I don't really care too much. I guess we can learn to grep for both symbols
instead of seeing the definition and usage in the same query, though I really
don't see why that's an advantage.

[pgervais, 2014-08-02 01:27:30]

On 2014/08/02 00:30:15, iannucci wrote:
> On 2014/08/02 00:27:52, pgervais wrote:
> > On 2014/08/02 00:25:01, pgervais wrote:
> > > On 2014/08/01 23:49:21, iannucci wrote:
> > > > https://codereview.chromium.org/436963005/diff/1/git_cache.py
> > > > File git_cache.py (right):
> > > > 
> > > > https://codereview.chromium.org/436963005/diff/1/git_cache.py#newcode545
> > > > git_cache.py:545: parser.add_option('--no_bootstrap',
action='store_true',
> > > > this one?
> > > > 
> > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py
> > > > File git_cl.py (right):
> > > > 
> > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1719
> > > > git_cl.py:1719: parser.add_option('-s', '--send-mail',
> action='store_true',
> > > > this should gain --send_mail
> > > > 
> > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1726
> > > > git_cl.py:1726: parser.add_option('-c', '--use-commit-queue',
> > > > action='store_true',
> > > > same
> > > > 
> > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1736
> > > > git_cl.py:1736: parser.add_option('--auto-bots', default=False,
> > > > action='store_true',
> > > > same
> > > > 
> > > > https://codereview.chromium.org/436963005/diff/1/git_new_branch.py
> > > > File git_new_branch.py (right):
> > > > 
> > > >
> https://codereview.chromium.org/436963005/diff/1/git_new_branch.py#newcode21
> > > > git_new_branch.py:21: g.add_argument('--upstream-current',
> > > action='store_true',
> > > > replaced, not added
> > > 
> > > I replaced it because I believe we should not use underscores in option
> names.
> > > I do not really buy your argument about grepping, because it only makes
> sense
> > > inside the python file.
> > > Having two long options which differ only by underscore / hyphen will lead
> to
> > > some confusion in calling code. We'll end up with half of the calling code
> > using
> > > one form, and the other half using the other one. So we'll have to grep
for
> > both
> > > forms anyway.
> > > 
> > > New patch on its way.
> > 
> > To be more precise: I *added* hyphenated options instead of replacing them
> > because I don't want to break existing calling code. But I'd like to clean
> that
> > up in the future. The only option I renamed I was pretty sure was not used
in
> > slaves (tell me if I'm wrong).
> 
> It's either used by slaves, and it will break, or it's used by devs, and they
> will probably be annoyed.

Is git new-branch possibly used by slaves? 
I guess it will annoy devs, but they're way better at recovering from failure
that bots :-)
 
> I don't really care too much. I guess we can learn to grep for both symbols
> instead of seeing the definition and usage in the same query, though I really
> don't see why that's an advantage.

My point is that having only one style for options is an advantage, because you
don't have to ask yourself which style is in use for this particular option when
calling the script. Having two styles at the same time will force use to type
the two definitions each time (unless we patch argparse, which I think we should
NOT do), and having two different ways of passing the same option will be
annoying. What if you have to de-duplicate an option? What if you want to verify
that an option has been passed (in the calling code)?  You'll have to convert to
one style first, then perform the operation. This is worst than having to run
grep twice, because it does not immediately show up, and will lead to bugs later
on.

Thus I'm advocating for only one style: the hyphenated one, because it's IMHO
the most common, and the easiest to type on both us and no-us layouts.

[iannucci, 2014-08-02 01:35:59]

On 2014/08/02 01:27:30, pgervais wrote:
> On 2014/08/02 00:30:15, iannucci wrote:
> > On 2014/08/02 00:27:52, pgervais wrote:
> > > On 2014/08/02 00:25:01, pgervais wrote:
> > > > On 2014/08/01 23:49:21, iannucci wrote:
> > > > > https://codereview.chromium.org/436963005/diff/1/git_cache.py
> > > > > File git_cache.py (right):
> > > > > 
> > > > >
https://codereview.chromium.org/436963005/diff/1/git_cache.py#newcode545
> > > > > git_cache.py:545: parser.add_option('--no_bootstrap',
> action='store_true',
> > > > > this one?
> > > > > 
> > > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py
> > > > > File git_cl.py (right):
> > > > > 
> > > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1719
> > > > > git_cl.py:1719: parser.add_option('-s', '--send-mail',
> > action='store_true',
> > > > > this should gain --send_mail
> > > > > 
> > > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1726
> > > > > git_cl.py:1726: parser.add_option('-c', '--use-commit-queue',
> > > > > action='store_true',
> > > > > same
> > > > > 
> > > > > https://codereview.chromium.org/436963005/diff/1/git_cl.py#newcode1736
> > > > > git_cl.py:1736: parser.add_option('--auto-bots', default=False,
> > > > > action='store_true',
> > > > > same
> > > > > 
> > > > > https://codereview.chromium.org/436963005/diff/1/git_new_branch.py
> > > > > File git_new_branch.py (right):
> > > > > 
> > > > >
> > https://codereview.chromium.org/436963005/diff/1/git_new_branch.py#newcode21
> > > > > git_new_branch.py:21: g.add_argument('--upstream-current',
> > > > action='store_true',
> > > > > replaced, not added
> > > > 
> > > > I replaced it because I believe we should not use underscores in option
> > names.
> > > > I do not really buy your argument about grepping, because it only makes
> > sense
> > > > inside the python file.
> > > > Having two long options which differ only by underscore / hyphen will
lead
> > to
> > > > some confusion in calling code. We'll end up with half of the calling
code
> > > using
> > > > one form, and the other half using the other one. So we'll have to grep
> for
> > > both
> > > > forms anyway.
> > > > 
> > > > New patch on its way.
> > > 
> > > To be more precise: I *added* hyphenated options instead of replacing them
> > > because I don't want to break existing calling code. But I'd like to clean
> > that
> > > up in the future. The only option I renamed I was pretty sure was not used
> in
> > > slaves (tell me if I'm wrong).
> > 
> > It's either used by slaves, and it will break, or it's used by devs, and
they
> > will probably be annoyed.
> 
> Is git new-branch possibly used by slaves? 
> I guess it will annoy devs, but they're way better at recovering from failure
> that bots :-)
>  
> > I don't really care too much. I guess we can learn to grep for both symbols
> > instead of seeing the definition and usage in the same query, though I
really
> > don't see why that's an advantage.
> 
> My point is that having only one style for options is an advantage, because
you
> don't have to ask yourself which style is in use for this particular option
when
> calling the script. Having two styles at the same time will force use to type
> the two definitions each time (unless we patch argparse, which I think we
should
> NOT do), and having two different ways of passing the same option will be
> annoying. What if you have to de-duplicate an option? What if you want to
verify
> that an option has been passed (in the calling code)?  You'll have to convert
to
> one style first, then perform the operation. This is worst than having to run
> grep twice, because it does not immediately show up, and will lead to bugs
later
> on.
> 
> Thus I'm advocating for only one style: the hyphenated one, because it's IMHO
> the most common, and the easiest to type on both us and no-us layouts.

Then let's do that, and I'll just deal with the greppage :).

[dnj, 2014-08-02 16:06:59]

+1 on only one expression of a command-line option. I also prefer underscores
for both 'grep' and in-editor searching. It's also more consistent with Google
tooling, from what I can tell.

However, if it's a burden to type on non-US layouts that seems like a pretty
good reason to use hyphens.

[pgervais, 2014-08-04 16:08:28]

On 2014/08/02 16:06:59, dnj wrote:
> +1 on only one expression of a command-line option. I also prefer underscores
> for both 'grep' and in-editor searching. It's also more consistent with Google
> tooling, from what I can tell.
> 
> However, if it's a burden to type on non-US layouts that seems like a pretty
> good reason to use hyphens.

I was stating the other way: an underscore is harder to type on a US layout than
an hyphen. 

It seems that you're right: Google tooling uses underscores rather than hyphens.
Then let's switch to underscores everywhere. I'm most of all advocating for
consistency here. Ii'll file a new patchset then.

[cmp, 2014-08-04 21:49:19]

+vadimsh since a couple of years ago I mentioned to him that we should pick one
or the other with the intention of getting all of these reasons out in the open
and then use these consistently.

[pgervais, 2014-08-06 02:18:50]

On 2014/08/04 21:49:19, cmp wrote:
> +vadimsh since a couple of years ago I mentioned to him that we should pick
one
> or the other with the intention of getting all of these reasons out in the
open
> and then use these consistently.

Since the team agreed on using hyphen-separated options, I fixed a few more
things in calling code. There is still work to do, but I think this is
reasonable for a first batch.

iannucci: I re-added --upstream_current in git_new_branch, for backward
compatibility. This CL should then be a perfect no-op to the infra.

[pgervais, 2014-08-07 16:45:05]

On 2014/08/06 02:18:50, pgervais wrote:
> On 2014/08/04 21:49:19, cmp wrote:
> > +vadimsh since a couple of years ago I mentioned to him that we should pick
> one
> > or the other with the intention of getting all of these reasons out in the
> open
> > and then use these consistently.
> 
> Since the team agreed on using hyphen-separated options, I fixed a few more
> things in calling code. There is still work to do, but I think this is
> reasonable for a first batch.
> 
> iannucci: I re-added --upstream_current in git_new_branch, for backward
> compatibility. This CL should then be a perfect no-op to the infra.

ping. Can we commit this before everything changes?

[iannucci, 2014-08-07 23:58:29]

lgtm

[pgervais, 2014-08-08 00:12:51]

The CQ bit was checked by pgervais@chromium.org

[commit-bot: I haz the power, 2014-08-08 00:13:30]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pgervais@chromium.org/436963005/40001

[commit-bot: I haz the power, 2014-08-08 00:14:06]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

Transient error: <urlopen error [Errno 104] Connection reset by peer>

[commit-bot: I haz the power, 2014-08-08 00:14:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 00:14:21]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

Transient error: <urlopen error [Errno 104] Connection reset by peer>

[pgervais, 2014-08-08 00:20:28]

The CQ bit was checked by pgervais@chromium.org

[commit-bot: I haz the power, 2014-08-08 00:21:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pgervais@chromium.org/436963005/40001

[commit-bot: I haz the power, 2014-08-08 00:25:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 00:25:28]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

Transient error: <urlopen error [Errno 104] Connection reset by peer>

[pgervais, 2014-08-08 00:44:01]

The CQ bit was checked by pgervais@chromium.org

[commit-bot: I haz the power, 2014-08-08 00:44:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pgervais@chromium.org/436963005/40001

[commit-bot: I haz the power, 2014-08-08 00:45:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 00:45:42]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

Transient error: <urlopen error [Errno 104] Connection reset by peer>

[sheyang, 2014-08-08 01:39:57]

The CQ bit was checked by sheyang@chromium.org

[commit-bot: I haz the power, 2014-08-08 01:40:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pgervais@chromium.org/436963005/40001

[commit-bot: I haz the power, 2014-08-08 01:41:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 01:42:10]

Presubmit check for 436963005-40001 failed and returned exit status 1.

Running presubmit commit checks ...
Checking out rietveld...
Running save-description-on-failure.sh
Running push-basic.sh
Running upstream.sh
Running submit-from-new-dir.sh
Running abandon.sh
Running submodule-merge-test.sh
Running upload-local-tracking-branch.sh
Running hooks.sh
Running post-dcommit-hook-test.sh
Running upload-stale.sh
Running patch.sh
Running basic.sh

** Presubmit ERRORS **
tests/gclient_smoketest.py (49.93s) failed
.......................................F......
======================================================================
FAIL: testSyncJobs (__main__.GClientSmokeSVN)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 451, in testSyncJobs
    untangle=True)
  File "tests/gclient_smoketest.py", line 86, in parseGclient
    return self.checkBlock(stdout, items)
  File "tests/gclient_smoketest.py", line 130, in checkBlock
    results = self.splitBlock(stdout)
  File "tests/gclient_smoketest.py", line 117, in splitBlock
    self.fail(line)
AssertionError: __main__.GClientSmokeSVN.testSyncJobs/src/other/DEPS

----------------------------------------------------------------------
Ran 46 tests in 49.709s

FAILED (failures=1)


Presubmit checks took 95.6s to calculate.

[pgervais, 2014-08-08 15:49:24]

The CQ bit was checked by pgervais@chromium.org

[commit-bot: I haz the power, 2014-08-08 15:50:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pgervais@chromium.org/436963005/40001

[commit-bot: I haz the power, 2014-08-08 15:52:36]

Change committed as 288366