Closed Bug 1186933 Opened 9 years ago Closed 9 years ago

Let's go back to just posting the links to the changesets on bugs when people land stuff

Categories

(Developer Services :: Mercurial: bzpost, defect)

Product:
Developer Services
Component:
Mercurial: bzpost
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: ehsan.akhgari, Unassigned)

References

Details

Attachments

(1 file)

MozReview Request: bzpost: make comments less verbose (bug 1186933); r?smacleod
40 bytes, text/x-review-board-request
smacleod
: review+
Details 
Pulsebot posts a single URL per patch that is pushed to an integration branch to the corresponding bug.  That is great!  Apparently bug 1177022 made it so that now in addition to the links that are helpful, you will get a ton of absolutely useless information.  Which makes bugs unnecessarily long, and will waste people's times.  I am spending minutes every day just ignoring these, and I think this needs to stop.

Take this for example: https://bugzilla.mozilla.org/show_bug.cgi?id=1186589#c5

Here is what you will see.  I will point out why everything there is pure noise:

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/213875242ee5636f0e052ede2da4a69a975ea096

Great!  (The "url:        " prefix is of questionable value, since most people will realize that they are looking at a URL.

changeset:  213875242ee5636f0e052ede2da4a69a975ea096

Redundant.  Literally repeating what's in the above line.

user:       Josh Matthews <josh@joshmatthews.net>

Redundant.  Literally repeating the person who the comment appears to be from.

date:       Thu Jul 23 10:25:12 2015 -0400

Redundant.  Literally repeating the date of the Bugzilla comment.

description:
Bug 1186589 - Ensure CORS preflight requests are never intercepted. r=sicking

Redundant.  Literally repeating the summary of the bug.

It's even more annoying when people write good commit messages.  And especially so when there are more than one patch pushed in the bug.  The commit message is available through clicking on the link, so I don't think repeating it in bug comments adds any value.

I think bug 1177022 needs to be backed out.
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #0)
> Pulsebot posts a single URL per patch that is pushed to an integration
> branch to the corresponding bug.  That is great!  Apparently bug 1177022
> made it so that now in addition to the links that are helpful, you will get
> a ton of absolutely useless information.  Which makes bugs unnecessarily
> long, and will waste people's times.  I am spending minutes every day just
> ignoring these, and I think this needs to stop.

personally I'm not a huge fan of even having links, but that's not a big deal.  This is taking up enough space that its actually a bit annoying.

> description:
> Bug 1186589 - Ensure CORS preflight requests are never intercepted. r=sicking
> 
> Redundant.  Literally repeating the summary of the bug.
> 
> It's even more annoying when people write good commit messages.  And
> especially so when there are more than one patch pushed in the bug.  The
> commit message is available through clicking on the link, so I don't think
> repeating it in bug comments adds any value.

and people often put commit messages in bug comments when they attach the patch, so it often is redundant with stuff in the bug too!
Flags: needinfo?(gps)
My intention of the more verbose comments left by bzpost was to make the version control information captured in Bugzilla more useful, enabling people to avoid having to incur a separate page view for certain tasks. It could also help verify correctness. e.g. if a commit lands on the wrong bug, the author and commit message should make that obvious sooner. In my mind, the URLs by themselves add little value.

While I concede the verbose comments aren't perfect, I've heard feedback that some prefer them to the plain URLs. It's obvious you can't please all the people all the time.

I don't think it is outside the realm of possibility to have Bugzilla do more work for us, possibly as configuration options, so more people can be pleased. For example:

* We can tag the commit-related comments as such and a) hide them b) toggle verbosity
* We can teach Bugzilla to add .onClick() or similar magic to commit URLs and have it dynamically load more detailed info via XHR to hg.mozilla.org
* We can aggregate all commits in a bug into a "commits table" and render metadata appropriately. I don't know how many times I've had to do dive through comments to find this stuff. (Of course, the repository is the better source of this info, but many people don't do that.)

mcote: I'm curious what your thoughts are about BMO turning commit URLs into something useful. I'd like to have a story for something better than commit URLs before we back out the verbose comments that bzpost is now leaving.
Flags: needinfo?(gps) → needinfo?(mcote)
(In reply to Gregory Szorc [:gps] from comment #2)
> My intention of the more verbose comments left by bzpost was to make the
> version control information captured in Bugzilla more useful, enabling
> people to avoid having to incur a separate page view for certain tasks. It
> could also help verify correctness. e.g. if a commit lands on the wrong bug,
> the author and commit message should make that obvious sooner. In my mind,
> the URLs by themselves add little value.

Well, in bug 1177022 comment 0 you suspected that the reason why we post the URLs and nothing else is not because it is too cumbersome for humans to do, but that's not true since for the past 3 years or so (ever since we have had https://github.com/mozilla/bugherder) these have been posted by bots, and not humans.

The reason why the URL is posted is for future reference.  And all of the information in the version control system is there for anyone who wants to use it.  Having that information being duplicated in the bug makes the bug harder to read by adding unnecessary noise.

> While I concede the verbose comments aren't perfect, I've heard feedback
> that some prefer them to the plain URLs. It's obvious you can't please all
> the people all the time.
> 
> I don't think it is outside the realm of possibility to have Bugzilla do
> more work for us, possibly as configuration options, so more people can be
> pleased. For example:
> 
> * We can tag the commit-related comments as such and a) hide them b) toggle
> verbosity

Hiding them is the wrong thing to do, since the information (the URL) is useful.  It is the extra copy of the information in the URL that is unhelpful.  That seems like the worst of both worlds.

> * We can teach Bugzilla to add .onClick() or similar magic to commit URLs
> and have it dynamically load more detailed info via XHR to hg.mozilla.org

What is wrong with not doing that, and have clicking on the link do exactly what clicking on every other link does?

> * We can aggregate all commits in a bug into a "commits table" and render
> metadata appropriately. I don't know how many times I've had to do dive
> through comments to find this stuff. (Of course, the repository is the
> better source of this info, but many people don't do that.)

I have always searched the page for "hg." and I can't remember ever having had difficulty finding the commits.  But sure, making the commits appear outside of comments could be fine, I guess, but what does bug 1177022 buy us in the mean time?!

> mcote: I'm curious what your thoughts are about BMO turning commit URLs into
> something useful. I'd like to have a story for something better than commit
> URLs before we back out the verbose comments that bzpost is now leaving.

Do we have any evidence that people find these links somehow insufficient?  It really seems that bug 1177022 has been a solution and now we're searching to see what problem it is solving.  That seems backward to me...
> mcote: I'm curious what your thoughts are about BMO turning commit URLs into
> something useful. I'd like to have a story for something better than commit
> URLs before we back out the verbose comments that bzpost is now leaving.

We can, of course, do any of that, but I'm not entirely sure how useful it either.  I thought we wanted to keep commit information in VCS and code-review tools and keep bugs for mainly discussing the problem and proposed solutions.
Flags: needinfo?(mcote)
(In reply to Mark Côté [:mcote] from comment #4)
> > mcote: I'm curious what your thoughts are about BMO turning commit URLs into
> > something useful. I'd like to have a story for something better than commit
> > URLs before we back out the verbose comments that bzpost is now leaving.
> 
> We can, of course, do any of that, but I'm not entirely sure how useful it
> either.  I thought we wanted to keep commit information in VCS and
> code-review tools and keep bugs for mainly discussing the problem and
> proposed solutions.

We do. But people cling to their old habits and like it or not, Bugzilla is pretty much the center of the Mozilla development universe.
So I guess it's back to Ehsan's question: how many people are getting value out of extra data, do you think?
Ping?
Flags: needinfo?(gps)
How about a compromise: we drop all the extra data *except* the first line of the commit message. I think the summary line adds important context that can be used to:

* Verify reviewer annotation is sane
* Verify commit message attributed the proper bug (helps detect typos in bug numbers faster)
* Clarifies which commits from a multi-commit review have landed

We could certainly bikeshed on formats. I'll throw out:

  https://hg.mozilla.org/integration/mozilla-inbound/rev/853e7b5582a855a0657750740aef6eee540187bb
  Bug 1186933 - Make bug comment summaries less verbose; r=ehsan
  
  https://hg.mozilla.org/integration/mozilla-inbound/rev/62cd40885e9362dc726f681675b64aa9577982aa
  Bug 1186933 - Some other commit referencing this bug; r=ehsan

(Also, I'd like us to start using the full 40 character hashes in more places. I'd appreciate if pulsebot switched at its earliest convenience.)
Flags: needinfo?(gps)
(In reply to Gregory Szorc [:gps] from comment #8)
> How about a compromise: we drop all the extra data *except* the first line
> of the commit message. I think the summary line adds important context that
> can be used to:
> 
> * Verify reviewer annotation is sane

What if it wasn't?  I don't remember ever seeing someone backing out something and relanding it if they get the reviewer wrong.  Has that ever happened?  Or has someone asked for it?

> * Verify commit message attributed the proper bug (helps detect typos in bug
> numbers faster)

If you get the bug # wrong, the comment will be posted to a different bug, so including the first line of commit message won't help you.  (And in fact we could just not do anything in bzpost and have pulsebot do what it does today and still get the same feature.)

> * Clarifies which commits from a multi-commit review have landed

This is fine.  But in practice, the check-in flag on the attachment is usually how people signal this and it is a much better way of doing this anyway.  (I realize that is hard to do from bzpost.)

> We could certainly bikeshed on formats. I'll throw out:
> 
>  
> https://hg.mozilla.org/integration/mozilla-inbound/rev/
> 853e7b5582a855a0657750740aef6eee540187bb
>   Bug 1186933 - Make bug comment summaries less verbose; r=ehsan
>   
>  
> https://hg.mozilla.org/integration/mozilla-inbound/rev/
> 62cd40885e9362dc726f681675b64aa9577982aa
>   Bug 1186933 - Some other commit referencing this bug; r=ehsan

I still think what pulsebot does for us today is really enough, but if the above hasn't convinced you, I don't know what will.  In that case, I guess this is the best outcome I can hope for, so I can live with it...  :/
This is another sad example of how bad this is: https://bugzilla.mozilla.org/show_bug.cgi?id=1144660#c19
bzpost: make comments less verbose (bug 1186933); r?smacleod

A number of people were voicing concerns that the extra data was too
verbose and often redundant. Let's trim down the size of the content to
just the URL and first line of the commit message.
Attachment #8645039 - Flags: review?(smacleod)
Comment on attachment 8645039 [details]
MozReview Request: bzpost: make comments less verbose (bug 1186933); r?smacleod

https://reviewboard.mozilla.org/r/15365/#review14651

Ship It!
Attachment #8645039 - Flags: review?(smacleod) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Thanks!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: