1328717 - Move buglink filter before firstline filter

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Gregory Szorc [:gps]]

In bug 1325782, we expanded the "buglink" template filter to hyperlink to GitHub when appropriate.

Within seconds of me showing the deployed shiny hotness to Manishearth, he noticed it wasn't hyperlinking correctly in the short messages.

The reason for this is that the hyperlinking filter is running on only the first line of the string. So, it doesn't see the annotation telling it to link to GitHub.

We can fix this by changing the order of the filters so buglink runs before firstline.

Comment 1

[Gregory Szorc [:gps]]

Attachment: hgmo: rename "buglink" to "mozlink" (bug 1328717);

The filter no longer links to just Bugzilla: it now links to many
things. So the name "buglink" is no longer appropriate. Plus,
Mercurial has a built-in "buglink" special variable in extensions,
that makes searching for occurrences of the filter harder than it
should be.

Rename the filter to "mozlink" for clarity.

Review commit: https://reviewboard.mozilla.org/r/102316/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102316/

Comment 2

[Gregory Szorc [:gps]]

Attachment: hgmo: test for mozlink after firstline filter (bug 1328717);

mozlink doesn't work correctly if it only operates on the first line of
a commit message.

So, we add a crude static analysis test that prints warnings for all
occurrences of this bug.

Review commit: https://reviewboard.mozilla.org/r/102318/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102318/

Comment 3

[Gregory Szorc [:gps]]

Attachment: hgtemplates: move mozlink filter before firstline (bug 1328717);

The mozlink filter needs full commit message context in order to
hyperlink properly. In a few occurrences, it was coming after the
firstline filter, preventing it from doing its job.

We simply reorder the filters to fix the issue. The static analysis
test now shows we are in compliance!

Review commit: https://reviewboard.mozilla.org/r/102320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102320/

Comment 4

[William Lachance (:wlach)]

Comment on attachment 8823832 [details]
hgmo: test for mozlink after firstline filter (bug 1328717);

https://reviewboard.mozilla.org/r/102318/#review102724

::: hgext/hgmo/tests/test-mozlink.t:63
(Diff revision 1)
>    Source-Repo: <a href="https://github.com/mozilla/foo">https://github.com/mozilla/foo</a>
> +
> +  $ cd ..
> +
> +Occurrences of the filter must occur *before* firstline otherwise it doesn't have
> +sufficient context to link appropriately.

A more detailed description of what and why we're testing here would be helpful. Even with the full context of this review, I was pretty confused about what was going on here.

Comment 5

[William Lachance (:wlach)]

Comment on attachment 8823831 [details]
hgmo: rename "buglink" to "mozlink" (bug 1328717);

https://reviewboard.mozilla.org/r/102316/#review102726

Comment 6

[William Lachance (:wlach)]

Comment on attachment 8823833 [details]
hgtemplates: move mozlink filter before firstline (bug 1328717);

https://reviewboard.mozilla.org/r/102320/#review102730

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8823832 [details]
hgmo: test for mozlink after firstline filter (bug 1328717);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102318/diff/1-2/

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8823833 [details]
hgtemplates: move mozlink filter before firstline (bug 1328717);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102320/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/34115287512c
hgmo: rename "buglink" to "mozlink" ; r=wlach
https://hg.mozilla.org/hgcustom/version-control-tools/rev/327e7ca92fe7
hgmo: test for mozlink after firstline filter ; r=wlach
https://hg.mozilla.org/hgcustom/version-control-tools/rev/1dcc7141886c
hgtemplates: move mozlink filter before firstline ; r=wlach