push hooks should print out hg url for the changeset

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
--
enhancement
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: heycam, Assigned: graememcc)

Tracking

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

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

Updated

6 years ago
Component: Release Engineering → Hg: Customizations
QA Contact: release → hg.customizations
(Assignee)

Comment 1

5 years ago
Created attachment 619044 [details] [diff] [review]
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.
Attachment #619044 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 2

5 years ago
Created attachment 619053 [details] [diff] [review]
Changegroup hook v1b

Nit: singular/plural for the printing revs case
Attachment #619044 - Attachment is obsolete: true
Attachment #619044 - Flags: review?(ted.mielczarek)
Attachment #619053 - Flags: review?(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.
Attachment #619053 - Flags: review?(ted.mielczarek) → review-
Duplicate of this bug: 734092
Apparently I liked this bug so much I filed a dupe! Thanks for writing a patch!
Assignee: nobody → graememcc_firefox
(Assignee)

Comment 6

5 years ago
Created attachment 620991 [details] [diff] [review]
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!
Attachment #619053 - Attachment is obsolete: true
Attachment #620991 - Flags: review?(ted.mielczarek)
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?
Attachment #620991 - Flags: review-
(Assignee)

Comment 8

5 years ago
Created attachment 621002 [details] [diff] [review]
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?
Attachment #620991 - Attachment is obsolete: true
Attachment #620991 - Flags: review?(ted.mielczarek)
Attachment #621002 - Flags: review?(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.
Attachment #621002 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 10

5 years ago
http://hg.mozilla.org/hgcustom/hghooks/rev/4ee58bd47d47
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

5 years ago
Evidently forgot to add the deployment bug dependency when I filed it.
Depends on: 751867
It would be handy to add aurora, beta, release and esr to this as well.
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.)
Done: http://hg.mozilla.org/hgcustom/hghooks/rev/fb2a907cea70
Blocks: 780815
Product: mozilla.org → Release Engineering
https://hg.mozilla.org/hgcustom/version-control-tools/rev/de4243a72d3b
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.