Re: Commit messages in Phabricator

2018-02-13 Thread Mark Côté
On Tuesday, 13 February 2018 14:05:55 UTC-5, Haik Aftandilian  wrote:
> On Mon, Feb 12, 2018 at 6:30 AM, Eric Rescorla  wrote:
> 
> > On Mon, Feb 12, 2018 at 6:09 AM, Boris Zbarsky  wrote:
> > Instead, maybe we can arrange for Phab/Lando to put the bug #in the short
> > message, potentially also with r=
> >
> 
> ​I agree. Having the bug ID on the first line of the commit message is
> essential when looking at hg log output and I think maintaining that is
> important. Having the reviewers is also useful.

Right, as Piotr said, this is what we intend to do with Lando.

Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Commit messages in Phabricator

2018-02-13 Thread Haik Aftandilian
On Mon, Feb 12, 2018 at 6:30 AM, Eric Rescorla  wrote:

> On Mon, Feb 12, 2018 at 6:09 AM, Boris Zbarsky  wrote:
> Instead, maybe we can arrange for Phab/Lando to put the bug #in the short
> message, potentially also with r=
>

​I agree. Having the bug ID on the first line of the commit message is
essential when looking at hg log output and I think maintaining that is
important. Having the reviewers is also useful.

Haik



>
> -Ekr
>
>
> >
> > -Boris
> >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Commit messages in Phabricator

2018-02-13 Thread lthomson
On Monday, February 12, 2018 at 1:48:54 PM UTC-5, Gregory Szorc wrote:
> While I'm not working on either client implementation and am not part of
> the Phabricator team, if someone wants to formalize the Mercurial or Git
> clients in version-control-tools before the Phabricator team has time to
> work on them, I'd be happy to review code or provide technical assistance.
> Track things against bug 1366401 if you do any work.

I suggest people hold off on doing any work here.

As gps notes, he is not working on either client implementation and is not part 
of the Phabricator team.

The team has been looking at alternatives for streamlining here, and while a 
similar proposal is on the list, it's not highly ranked and is not a likely 
choice. Work done at this stage would most likely be wasted.

Best

Laura
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Commit messages in Phabricator

2018-02-12 Thread Gregory Szorc
On Mon, Feb 12, 2018 at 6:09 AM, Boris Zbarsky  wrote:

> On 2/11/18 3:57 PM, Emilio Cobos Álvarez wrote:
>
>> Arc wants to use something like:
>>
>
> So from my point of view, having the bug# easily linked from various
> places where the short summary is all that's shown (pushlogs especially) is
> pretty useful.  It saves loading a bunch of extra things when trying to go
> from regression-range pushlogs to the relevant bugs


It's generally pretty easy to modify tooling to find linked bugs. We have a
shared Python module for parsing commit messages into useful metadata and
that's used by various tools for extracting bugs, reviewers, so they can be
rendered in various places (
https://hg.mozilla.org/hgcustom/version-control-tools/file/3743b6c62d05/pylib/mozautomation/mozautomation/commitparser.py).
And updating templating on hg.mozilla.org to render useful fields more
prominently is generally pretty turnkey. Please file bugs in the
hg.mozilla.org component if you want the display of things tweaked!

More to the point of the original question, there are several reasons why
we don't want to use Arcanist (`arc`) for Firefox development (and probably
more broadly at Mozilla). The initial comment in bug 1366401 records a lot
of them. The plan of record is to author a minimal client for submitting
reviews via Phabricator's HTTP API and to plumb that up to `mach` as
needed. This work isn't part of the Phabricator MVP, which is why the
Phabricator team hasn't worked on it.

If you use Mercurial, there is an alternative to Arcanist available today!
You can combine Mercurial's "phabricator" extension with a minimal wrapper
that Tom Prince wrote so it recognizes Mozilla's bug numbers and reviewer
annotations. Instructions are at
https://bugzilla.mozilla.org/show_bug.cgi?id=1366401#c4. This will likely
serve as the base for the eventual solution. It is probably less effort to
configure this than to install Arcanist. It is unsupported, but I think it
is better than using Arcanist.

Git users will likely have to wait until after the Phabricator MVP is
launched before there is staffing to work on a client. We kind of lucked
out that Mercurial had something that was almost drop-in ready for us to
consume and that is why there is a Mercurial alternative (i.e. this isn't
about prioritizing Mercurial over Git).

While I'm not working on either client implementation and am not part of
the Phabricator team, if someone wants to formalize the Mercurial or Git
clients in version-control-tools before the Phabricator team has time to
work on them, I'd be happy to review code or provide technical assistance.
Track things against bug 1366401 if you do any work.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Commit messages in Phabricator

2018-02-12 Thread Eric Rescorla
On Mon, Feb 12, 2018 at 6:09 AM, Boris Zbarsky  wrote:

> On 2/11/18 3:57 PM, Emilio Cobos Álvarez wrote:
>
>> Arc wants to use something like:
>>
>
> So from my point of view, having the bug# easily linked from various
> places where the short summary is all that's shown (pushlogs especially) is
> pretty useful.  It saves loading a bunch of extra things when trying to go
> from regression-range pushlogs to the relevant bugs


Yes, this does seem convenient. I imagine we could develop tooling to deal
with other locations, but probably better not to.

Instead, maybe we can arrange for Phab/Lando to put the bug #in the short
message, potentially also with r=

-Ekr


>
> -Boris
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Commit messages in Phabricator

2018-02-12 Thread Boris Zbarsky

On 2/11/18 3:57 PM, Emilio Cobos Álvarez wrote:

Arc wants to use something like:


So from my point of view, having the bug# easily linked from various 
places where the short summary is all that's shown (pushlogs especially) 
is pretty useful.  It saves loading a bunch of extra things when trying 
to go from regression-range pushlogs to the relevant bugs


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Commit messages in Phabricator

2018-02-12 Thread Piotr Zalewa
Hi Emilio,

Landing using upcoming "Lando" will use the following format
(taken from
https://github.com/mozilla-conduit/lando-api/blob/master/landoapi/commit_message.py#L21)
:

```
 -  r=,r=



Differential Revision: 
```

"Test Plan" is describing the steps to test the patch by the reviewer.

"Subscribers" is a list of users author would like to subscribe to the
revision.

zalun

On Sun, Feb 11, 2018 at 9:57 PM Emilio Cobos Álvarez 
wrote:

> Hey,
>
> So I've been testing Phabricator for a while and I've noticed that the
> arc tool uses different formatting for the commit messages than what
> usually lands on mozilla-central.
>
> In particular I usually lay out my commits like:
>
> ---
> Bug XXX: Short summary. r=
>
> Long description, if needed.
> ---
>
> Arc wants to use something like:
>
> ---
> Short summary.
>
> Summary:
> Long description (I assume?).
>
> Test Plan: (what goes here?)
>
> Reviewers: 
>
> Subscribers: (what goes here?)
>
> Bug #: XXX
>
> Differential revision: https://phabricator.services.mozilla.com/D574
> ---
>
> I've been manually fixing them up before committing them so they have
> the usual format when landing.
>
> My question is: Are commits that are going to land to autoland from
> phabricator directly going to land with the new format?
>
> Should we keep fixing them up before pushing to autoland / inbound, or
> should we start pushing commits with the new commit message formatting?
>
> Also, knowing an answer for the (what goes here?) questions would be
> super-helpful.
>
> Thanks a lot!
>
>   -- Emilio
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform