Commit messages in Phabricator

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=<reviewers>

Long description, if needed.
---

Arc wants to use something like:

---
Short summary.

Summary:
Long description (I assume?).

Test Plan: (what goes here?)

Reviewers: <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
0
UTF
2/11/2018 8:57:10 PM
mozilla.dev.platform 6467 articles. 0 followers. Post Follow

6 Replies
66 Views

Similar Articles

[PageSpeed] 43

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
0
Boris
2/12/2018 2:09:26 PM
On Mon, Feb 12, 2018 at 6:09 AM, Boris Zbarsky <bzbarsky@mit.edu> wrote:

> On 2/11/18 3:57 PM, Emilio Cobos =C3=81lvarez 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 g=
o
> 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=3D<foo>

-Ekr


>
> -Boris
>
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
0
Eric
2/12/2018 2:30:20 PM
On Mon, Feb 12, 2018 at 6:09 AM, Boris Zbarsky <bzbarsky@mit.edu> wrote:

> On 2/11/18 3:57 PM, Emilio Cobos =C3=81lvarez 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 g=
o
> 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/pyl=
ib/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=3D1366401#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.
0
Gregory
2/12/2018 6:48:02 PM
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
0
lthomson
2/13/2018 6:42:41 PM
On Mon, Feb 12, 2018 at 6:30 AM, Eric Rescorla <ekr@rtfm.com> wrote:

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

=E2=80=8BI 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
>
0
Haik
2/13/2018 7:05:45 PM
On Tuesday, 13 February 2018 14:05:55 UTC-5, Haik Aftandilian  wrote:
> On Mon, Feb 12, 2018 at 6:30 AM, Eric Rescorla <ekr@rtfm.com> wrote:
>=20
> > On Mon, Feb 12, 2018 at 6:09 AM, Boris Zbarsky <bzbarsky@mit.edu> wrote=
:
> > Instead, maybe we can arrange for Phab/Lando to put the bug #in the sho=
rt
> > message, potentially also with r=3D<foo>
> >
>=20
> =E2=80=8BI agree. Having the bug ID on the first line of the commit messa=
ge 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
0
UTF
2/13/2018 7:40:51 PM
Reply: