Last Comment Bug 663585 - push hooks should print out hg url for the changeset
: push hooks should print out hg url for the changeset
Status: RESOLVED FIXED
:
Product: Developer Services
Classification: Other
Component: Mercurial: hg.mozilla.org (show other bugs)
: other
: All All
: -- enhancement
: ---
Assigned To: Graeme McCutcheon [:graememcc]
:
Mentors:
: 734092 (view as bug list)
Depends on: 751867
Blocks: 780815
  Show dependency treegraph
 
Reported: 2011-06-10 18:41 PDT by Cameron McCormack (:heycam) (away Jun 25 – Jul 10)
Modified: 2014-10-02 07:01 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Changegroup hook v1 (4.79 KB, patch)
2012-04-27 08:19 PDT, Graeme McCutcheon [:graememcc]
no flags Details | Diff | Review
Changegroup hook v1b (4.84 KB, patch)
2012-04-27 08:39 PDT, Graeme McCutcheon [:graememcc]
ted: review-
Details | Diff | Review
Per comments (1.66 KB, patch)
2012-05-04 02:38 PDT, Graeme McCutcheon [:graememcc]
bugspam.Callek: review-
Details | Diff | Review
Per comments (1.65 KB, patch)
2012-05-04 03:58 PDT, Graeme McCutcheon [:graememcc]
ted: review+
Details | Diff | Review

Description Cameron McCormack (:heycam) (away Jun 25 – Jul 10) 2011-06-10 18:41:51 PDT
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] 2012-04-27 08:19:42 PDT
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.
Comment 2 Graeme McCutcheon [:graememcc] 2012-04-27 08:39:57 PDT
Created attachment 619053 [details] [diff] [review]
Changegroup hook v1b

Nit: singular/plural for the printing revs case
Comment 3 Ted Mielczarek [:ted.mielczarek] 2012-05-03 12:58:22 PDT
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 4 Ted Mielczarek [:ted.mielczarek] 2012-05-03 12:59:35 PDT
*** Bug 734092 has been marked as a duplicate of this bug. ***
Comment 5 Ted Mielczarek [:ted.mielczarek] 2012-05-03 13:00:15 PDT
Apparently I liked this bug so much I filed a dupe! Thanks for writing a patch!
Comment 6 Graeme McCutcheon [:graememcc] 2012-05-04 02:38:10 PDT
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!
Comment 7 Justin Wood (:Callek) 2012-05-04 03:04:20 PDT
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] 2012-05-04 03:58:50 PDT
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?
Comment 9 Ted Mielczarek [:ted.mielczarek] 2012-05-04 05:09:28 PDT
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] 2012-05-04 05:16:10 PDT
http://hg.mozilla.org/hgcustom/hghooks/rev/4ee58bd47d47
Comment 11 Graeme McCutcheon [:graememcc] 2012-05-22 08:56:37 PDT
Evidently forgot to add the deployment bug dependency when I filed it.
Comment 12 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-12 10:32:59 PDT
It would be handy to add aurora, beta, release and esr to this as well.
Comment 13 Ted Mielczarek [:ted.mielczarek] 2012-06-12 11:10:44 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-12 11:26:37 PDT
Done: http://hg.mozilla.org/hgcustom/hghooks/rev/fb2a907cea70

Note You need to log in before you can comment on or make changes to this bug.