Closed Bug 1508002 Opened 1 year ago Closed 1 year ago

Provide a hyper-blame like tool: hg smart-annotate

Categories

(Developer Services :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Felipe, Assigned: Felipe)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

Attachments

(1 file)

There was a conversation today on IRC about using the new `hg annotate --skip` to set up some structure and conventions around annotating csets to be skipped for blame.

This bug intends to define those conventions and to provide a tool to locally run annotate while skipping the defined changesets.


From IRC, I think we ended up with:

- csets with "skip-blame" in the commit message will be skipped
- past csets (or ones that missed that message) can be listed in a .hg-blame-ignore-revs file at the root of the repo


I'm naming it hyper-blame in reference to an equally-named tool for git. Let me know if we should choose something else.


There's bug 1376298, but I'm not making this a dupe of that one because that one is meant to do support this on hg.m.o. Once this bug is done, the work can continue there.
FWIW I've heard various people in the larger VCS space remark that "annotate" should be used instead of "blame" because it has less negative connotations. Blame sounds like you are looking to cast fault whereas annotate sounds like a neutral archeological term.

Mercurial's canonical command is actually "annotate" and "blame" is an alias. In Git, "annotate" and "blame" are different commands. (The term "blame" has its origins before Mercurial and Git.)

I would encourage us to adopt the less negative terminology and use "annotate" instead of "blame" for both the commit message "ignore this commit" annotation and for any aliases we install in people's configs.
How about "smart-annotate" for the alias? Another option would be to not add the alias at all, and just let people use the revsetalias

(I also thought about adding another revsetalias, ``backouts = desc('re:^backout')``, to be used independently)


The annotation in the commit message was following what the Mercurial project appears to be using (i.e., "# skip-blame"), although granted it's only been used a few times so far.

How about "skip-this-commit"?
(In reply to :Felipe Gomes (needinfo me!) from comment #3)
> The annotation in the commit message was following what the Mercurial
> project appears to be using (i.e., "# skip-blame"), although granted it's
> only been used a few times so far.
> 
> How about "skip-this-commit"?

I agree with the point in comment 2, but please note that https://hg.mozilla.org/mozilla-central/rev/0ceae9db9ec0 was landed with the bytes "# skip-blame" in the commit message.

But looking at the patch, how about not prescribing anything in the commit message and just sticking in the sha1s of the desired commits in the .hg-blame-ignore-revs file (which also doesn't adhere to comment 2...)?
Another idea is for the terminology to reflect what was done rather than what should be done. e.g. instead of "revisions to skip annotate for" we would annotate "revisions that are format-only rewrites." This more accurately describes what was done and doesn't prescribe behavior after-the-fact.
Sounds great to me!

(In the light of bug 1508324 which includes more than just format-only rewrites, we could possibly phrase it as "revisions that are automated rewrites" too.)
Attachment #9025835 - Attachment description: Bug 1508002 - Provide a hyper-blame command to hg to skip ignored changesets when viewing blame. r=gps → Bug 1508002 - Provide a smart-annotate command to hg to make annotate skip some chosen changesets. r=sheehan
So what I ended up with was:

alias: hg smart-annotate

filename: .hg-annotate-ignore-revs

I kept the part that ignores changesets based on the commit summary, using "ignore-this-changeset". If we prefer to not use it for mozilla-central, that's fine, we can just use the hashes in the file. But I think it's nicer to have something directly tied to the commit, instead of having to track it separately in a different place
Summary: Provide a hyper-blame like tool → Provide a hyper-blame like tool: hg smart-annotate
Yes, "ignore-this-changeset" sounds great to me!
Andi volunteered to push this to v-c-t to me. I think this is r+ already, sheehan's only concerns were the unrelated test fix (that I split to a separate bug) and properly sending this to v-c-t from phabricator
Flags: needinfo?(bpostelnicu)
hm....the patch is not r+, am i missing something extremely obvious here?
Flags: needinfo?(bpostelnicu)
Attachment #9025835 - Attachment description: Bug 1508002 - Provide a smart-annotate command to hg to make annotate skip some chosen changesets. r=sheehan → configwizard: provide a smart-annotate command to hg to make annotate skip some chosen changesets (Bug 1508002) r=sheehan
Pushed by cosheehan@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/746c4e252234
configwizard: provide a smart-annotate command to hg to make annotate skip some chosen changesets r=sheehan
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
I changed the repo to version-control-tools in Phabricator and landed this just now. :)

Looking forward to using smart-annotate!
Thanks!
Depends on: 1517657
Blocks: 1517727
You need to log in before you can comment on or make changes to this bug.