663585 - push hooks should print out hg url for the changeset

Status: RESOLVED FIXED

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

Description

[Cameron McCormack (:heycam)]

It'd be handy if, when pushing, the hg changeset URL were printed, since often you want to paste that URL into a bug.

Comment 1

[Graeme McCutcheon [:graememcc]]

Attachment: Changegroup hook v1

The ui.pushbuffer() and ui.popbuffer() methods make me think that I should be able to capture the output of pushing, and thus write a test, but I was never able to capture more than the "Pushing to..." message, despite various attempts.

Comment 2

[Graeme McCutcheon [:graememcc]]

Attachment: Changegroup hook v1b

Nit: singular/plural for the printing revs case

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 619053 [details] [diff] [review]
Changegroup hook v1b

Review of attachment 619053 [details] [diff] [review]:
-----------------------------------------------------------------

Mostly good, just one big hangup.

::: mozhghooks/push_printurls.py
@@ +49,5 @@
> +    'tracemonkey'        : 'https://hg.mozilla.org/tracemonkey/',
> +    'try'                : 'https://hg.mozilla.org/try/',
> +    'try-comm-central'   : 'https://hg.mozilla.org/try-comm-central/',
> +    'ux'                 : 'https://hg.mozilla.org/projects/ux/',
> +    'venkman'            : 'https://hg.mozilla.org/venkman/',

This is sort of horrible. Can we make this suck less somehow? I dread having to keep this list up-to-date. Maybe we can only handle really popular repos like m-c, inbound and try?

@@ +55,5 @@
> +
> +
> +def hook(ui, repo, node, hooktype, **kwargs):
> +    repo_name = os.path.basename(repo.root)
> +    if not hgNameToRevURL.has_key(repo_name):

if repo_name not in hgNameToRevURL:

@@ +67,5 @@
> +    url = hgNameToRevURL[repo_name]
> +
> +    if num_changes <= 10:
> +        plural = 's' if num_changes > 1 else ''
> +        print "You can view your changes at the following URL%s:" % plural

nit: use plural for "changes"

@@ +69,5 @@
> +    if num_changes <= 10:
> +        plural = 's' if num_changes > 1 else ''
> +        print "You can view your changes at the following URL%s:" % plural
> +        for i in xrange(rev, tip + 1):
> +            node = hex(repo.changectx(i).node())[:12]

You want mercurial.node.short here, not .hex, FYI.

@@ +70,5 @@
> +        plural = 's' if num_changes > 1 else ''
> +        print "You can view your changes at the following URL%s:" % plural
> +        for i in xrange(rev, tip + 1):
> +            node = hex(repo.changectx(i).node())[:12]
> +            print "  %srev/%s" % (url, node)

You could probably do this as "\n".join(a list comprehension), but I'm not sure if it'd be any more readable.

@@ +72,5 @@
> +        for i in xrange(rev, tip + 1):
> +            node = hex(repo.changectx(i).node())[:12]
> +            print "  %srev/%s" % (url, node)
> +    else:
> +       tip_node = hex(repo.changectx(tip).node())[:12]

short here as well.

Comment 5

[(not currently active) Ted Mielczarek]

Apparently I liked this bug so much I filed a dupe! Thanks for writing a patch!

Comment 6

[Graeme McCutcheon [:graememcc]]

Attachment: Per comments

> This is sort of horrible. Can we make this suck less somehow? I dread having
> to keep this list up-to-date. Maybe we can only handle really popular repos
> like m-c, inbound and try?

Agreed. Though I did add comm and fx-team in case they request the hook.

I was also half tempted to change the logic for try to spit out a TBPL URL for the build instead. 

>You want mercurial.node.short here, not .hex, FYI.

Didn't know about short, thanks for the tip!

Comment 7

[Justin Wood (:Callek)]

Comment on attachment 620991 [details] [diff] [review]
Per comments

Comm is not under releases there... i do wonder though if we cant get this repo path dynamically.

What happens if i push to a user repo named mozilla-central?

Comment 8

[Graeme McCutcheon [:graememcc]]

Attachment: Per comments

Gah. Typo, sorry.

> i do wonder though if we cant get this repo path dynamically.
> What happens if i push to a user repo named mozilla-central?

AFAICT the only route would be to further pass repo.name. From reading other bugs where repos were created, it looks like the main repos live under hg/mozilla/reponame but I wasn't clear if things like projects, automation etc. are in a path like hg/mozilla/projects or hg/projects. I went for the simpler approach to ensure I would have confidence my local testing would translate to hg.mo.o. This is the same approach used by the tree closure hook, for example.

Fair point on user repos, but how likely are they to have the hook activated?

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 621002 [details] [diff] [review]
Per comments

Review of attachment 621002 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! You'll need to file a separate Server Ops bug to get this enabled on various repos.

Comment 10

[Graeme McCutcheon [:graememcc]]

http://hg.mozilla.org/hgcustom/hghooks/rev/4ee58bd47d47

Comment 11

[Graeme McCutcheon [:graememcc]]

Evidently forgot to add the deployment bug dependency when I filed it.

Comment 12

[(no longer active)]

It would be handy to add aurora, beta, release and esr to this as well.

Comment 13

[(not currently active) Ted Mielczarek]

If you want more repos there, feel free to just push that change. (You'd need to update the deployment bug or file a new one, though.)

Comment 14

[(no longer active)]

Done: http://hg.mozilla.org/hgcustom/hghooks/rev/fb2a907cea70

Comment 15

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/de4243a72d3b