PSA: changes to gcl upload and git cl upload command line args

[Roger Tawa]

Hi all,

Summary
The meaning of -m/--message will soon change from "set patchset title"
to "publish a review message and send email" as if you were using the
Publish+Mail Comments form on rietveld. The new way to set the
patchset title is to use -t/--title. Please start using -t now instead
of -m.

Long version
Last week I merged upstream changes from rietveld's default branch
into the chromium branch which changed the interpretation of the
-m/--message command line arguments, see
http://codereview.appspot.com/6139060/ for complete details. Before
this change was merged, you could use the -m/--message command line
argument to set the title of a patchset when uploading to an existing
issue. After this change, the -m/--message argument is used instead
to publish a review request message to the issue. This merge did not
affect uploading to a new issue.

The result is that using commands like:

gcl upload foo -m title
git cl upload -m title

now sets the title of the patchset, publishes a review message to the
issue, and sends an email email to reviewers/cc.

For now, I have hacked gcl.py and git_cl.py to map the -m/--message to
-t/--title, so you don't need to change anything. (This will be
committed shortly, sorry for any inconvenience this has caused you).
Both -m/--message and -t/--title will do the same thing when
uploading. In the long term though, to keep gcl/git cl upload from
diverging from rietveld's upload.py, and to make sure we can support
publishing a review message at the same time as patchset upload, this
hack will be removed.

My plan is to leave both command line arguments in place for the next
month or so, to let everyone get their muscle memory of typing t
instead of m, and to update any personal scripts you might have. After
that, the command line arguments will be interpreted the same way as
upload.py:

-t --title TITLE New issue subject or new patch set title
-m --message MESSAGE New issue description or new patch set message

Let me know if you have any questions.

Thanks,
Roger

[Gabriel Charette]

Would be nice not to get the warning about -t if we are actually using it :)

--------------------------------------------------------------------------------------------------------

G:\src\component\src>git cl upload -t "fix gyp scope" -m "PTAL -- better scope for conditions in gyp"

WARNING: Use -t or --title to set the title of the patchset.

In the near future, -m or --message will send a message instead.

See http://goo.gl/JGg0Z for details.

--------------------------------------------------------------------------------------------------------

Cheers,

Gab

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

[Roger Tawa]

Hi Gab,

Until I take out my hack, -m is the same as -t, so there is no added
benefit to specifying both.

Thanks,
Roger

-

[Nico Weber]

Do you know what motivated the upstream change? The flag used to match svn / git.

Thanks,

Nico

[Mark Seaborn]

On 14 May 2012 09:01, Roger Tawa <rog...@chromium.org> wrote:

Hi all,

Summary
The meaning of -m/--message will soon change from "set patchset title"
to "publish a review message and send email" as if you were using the
Publish+Mail Comments form on rietveld. The new way to set the
patchset title is to use -t/--title. Please start using -t now instead
of -m.

Long version
Last week I merged upstream changes from rietveld's default branch
into the chromium branch which changed the interpretation of the
-m/--message command line arguments, see
http://codereview.appspot.com/6139060/ for complete details.  Before
this change was merged, you could use the -m/--message command line
argument to set the title of a patchset when uploading to an existing
issue.  After this change, the -m/--message argument is used instead
to publish a review request message to the issue.  This merge did not
affect uploading to a new issue.

I have a script (nacl_deps_bump.py) which uses -m to set the description of a newly-uploaded code review.  Will that still work with -m?  Your comment about uploading to a new issue suggests it will still work.

"git cl" currently has an option --send-mail.  Does the upcoming change mean that "git cl" will send e-mail even if --send-mail is not passed?  It would be odd if -m sometimes sends e-mail and sometimes doesn't.  I wouldn't want my script to accidentally spam people by sending out e-mail unintentionally when used in the wrong context.

Cheers,
Mark

[Elliot Glaysher (Chromium)]

> On Mon, May 14, 2012 at 9:01 AM, Roger Tawa <rog...@chromium.org> wrote:
>> My plan is to leave both command line arguments in place for the next
>> month or so, to let everyone get their muscle memory of typing t
>> instead of m, and to update any personal scripts you might have. After
>> that, the command line arguments will be interpreted the same way as
>> upload.py:

On Tue, May 15, 2012 at 4:54 PM, Nico Weber <tha...@chromium.org> wrote:
> Do you know what motivated the upstream change? The flag used to match svn /
> git.

After a few weeks, I'm still typing -m most of the time because
git/svn already use -m to set a commit message on my individual git
commits. Could we please keep the -m flag? Using -t doesn't match
git/svn, especially since we already have a --send-mail option.

-- Elliot

[Roger Tawa]

Hi Mark, Elliot,

Yes -m will still work to set the description for newly uploaded CLs.

Sorry, there was an error in my initial message: when I said "send
email" I meant to say "send chat notification". So once I remove the
hack, -m will be spammier than -t in the sense that it will add
messages to the CL and send chats to owner/reviewers/cc.

Looking back at the merge, the idea is to allow adding a message to
the CL at the same time as uploading a patchset to an existing CL. It
seems sending chats came along for the ride. Before I undo the hack,
I could change the server so that -m still adds a message to the CL,
but to send emails or chats you need to specify --send-mail. Does
that work for you?

Thanks,
Roger

-

[Elliot Glaysher (Chromium)]

On Mon, Jun 4, 2012 at 12:04 PM, Roger Tawa <rog...@chromium.org> wrote:
> Looking back at the merge, the idea is to allow adding a message to
> the CL at the same time as uploading a patchset to an existing CL.  It
> seems sending chats came along for the ride.  Before I undo the hack,
> I could change the server so that -m still adds a message to the CL,
> but to send emails or chats you need to specify --send-mail.  Does
> that work for you?

I'm still not exactly sure what you're proposing. If you mean the -m
command will set the patch set title without sending mails or chats,
then yes, that's what I want.

-- Elliot

[Roger Tawa]

Hi all,

Here is an *old* task that fell off my radar and I never completed
(thanks Tommi for the reminder). I plan to complete it in a couple of
weeks, which essentially means taking out the hack. All comments from
earlier have been addressed.

Let me know if you have any further questions.

Thanks,
Roger

-

[Isaac Levy]

This seems like a good change, but git-cl already has a send message argument (-s / --send-mail).  Why not use that instead of -m?

--

[Roger Tawa]

Hi all,

The removal of the hack was committed as crrev.com/202864. If you
have any problems with git cl upload, please let me know.

Thanks,
Roger

-

[Nico Weber]

On Wed, May 29, 2013 at 7:36 AM, Roger Tawa <rog...@chromium.org> wrote:

Hi all,

The removal of the hack was committed as crrev.com/202864.  If you
have any problems with git cl upload, please let me know.

This breaks the workflow I've used for the last few years without any upside. If I read this thread right, the reasoning is "this changed upstream, we don't know why". Is there any cost in keeping the hack around?

 

Thanks,
Roger

-

[Roger Tawa]

Hi Nico,

How does this break your workflow? Anywhere you did "git cl upload
-m" you now can do "git cl upload -t".

This change has been coming for more than a year now :-) I even
addressed your comments from 2012-05-12. Why would we want to keep a

hack around?

Thanks,
Roger

-

[Nico Weber]

On Thu, May 30, 2013 at 10:55 AM, Roger Tawa <rog...@chromium.org> wrote:

Hi Nico,

How does this break your workflow?  Anywhere you did "git cl upload
-m" you now can do "git cl upload -t".

Right, I need to change what I type. And now I have to be careful to use -t for git cl upload but -m for git commit.

 

This change has been coming for more than a year now :-)  I even
addressed your comments from 2012-05-12.  Why would we want to keep a

hack around?

One man's hack is another man's UI improvement I guess?

[Steve VanDeBogart]

It's not clear what's happened.  Does -m now send a message?

Roger's message from 6/4/12 (and erg's reply on 6/6/12) implied that the behavior of -m would not change unless --send-mail was also specified. The behavior of git cl upload -m MSG should not change since it is ingrained into so many people's muscle memory. 

[Thiago Farina]

I got used to git cl upload -t"MY PATCH SET MESSAGE". But it's a pain
since I use git ci -m"MY SHORT COMMIT MESSAGE". That seems to be
Nico's complain.

--
Thiago

[Roger Tawa]

Hi guys,

git cl upload is a wrapper around rietveld upload.py. The reason for
this change is to keep up with capabilities of upload.py and maintain
consistency with its command line args. We could certainly choose not
to do this, but as upload.py evolves this may/will get harder. This
is not a theoretical argument: witness all the recent changes to
rietveld.

If the only price to pay is remembering to type -t instead of -m,
seems well worth it to me. But if the chromium team feels this is not
an acceptable price to pay, I'll undo the hack removal.

Thanks,
Roger

-

[Marc-Antoine Ruel]

I approve Roger's change.

Being able to specify both a patchset title and sending a message simultaneously is useful.

But more importantly, having the -m flag do different things based on context of other flags significantly increases the learning curve. I agree this is a real annoyance to change a flag purpose but keeping confusing flag behavior for historical reasons is quite toxic on the long term. git has many examples of this.

While we could put an analogy between git commit -m and git cl upload -t vs -m, I don't think this is a not a great argument. I use several commands with several different flag names and I survive. Keep in mind all of this is optional.

M-A

2013/5/31 Roger Tawa <rog...@chromium.org>