1045602 - The page title for individual revisions should include details (port bug 1020919 to treeherder)

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P4)

Description

[:Paolo Amadini]

When viewing an individual revisions, the page title should display the revision number, followed by the main bug number from bug 1045600 when available. The job count summaries would also be useful as part of the details.

This can be considered even for other views, showing details from the first row.

This is especially useful with "side tabs" add-ons that provide more space for reading the title.

Comment 1

[Ed Morley [:emorley]]

Is the solution used by TBPL's recent bug 1020919 something that sounds reasonable?

Comment 2

[:Paolo Amadini]

(In reply to Ed Morley [:edmorley] from comment #1)
> Is the solution used by TBPL's recent bug 1020919 something that sounds
> reasonable?

Yeah, the format used there looks good! The revision number could still be included at the end to distinguish different pushes of slightly different patches, but I don't think it's a requirement.

Comment 3

[Ed Morley [:emorley]]

(In reply to :Paolo Amadini from comment #2)
> Yeah, the format used there looks good! The revision number could still be
> included at the end to distinguish different pushes of slightly different
> patches, but I don't think it's a requirement.

I think people are unlikely to remember the SHA for each push, and I think in this instance less is more. However we can always tweak in the future, but for now let's just not regress bug 1020919 in the switch from TBPL to treeherder (porting bug 1020919 would be a blocker for switching; enhancing it a feature request for later etc).

Comment 5

[Ed Morley [:emorley]]

TBPL's implementation of deriving a sensible tab title is at:
https://hg.mozilla.org/webtools/tbpl/file/8aff676182f9/js/UserInterface.js#l232

The Treeherder title is set here:
https://github.com/mozilla/treeherder/blob/5afdae3a9df466496dedb2022e61f4d9b4d971c0/ui/js/controllers/main.js#L24

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: PR 683

Comment 7

[Ed Morley [:emorley]]

Comment on attachment 8626742 [details] [review]
PR 683

Thank you for looking at this :-)

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8626742 [details] [review]
PR 683

Think this is ready for a final review. Nesting has been reduced. The console.log calls have been removed. It fetches the commit info from the datamodel, not via page scraping.

(I also fixed a thing that annoyed me where the title would show "undefined" for a bit right as the page is loading before the repo name is loaded (for both specific revisions and the general repo view).)

Comment 9

[Ed Morley [:emorley]]

Just as a heads up, I'm not going to be able to finish the review before next week - since it's going to take a bit longer to remind myself about the PR since it's been 3 weeks since the last review request. Will give it a spin next week and hopefully we can get it landed shortly after :-)

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I went ahead and added the percent complete that was requested in bug 1188027 in this PR. Unsure what the best arrangement of the items in the title should be. It's currently "[0] 5% - Bug Title".

Comment 12

[Ed Morley [:emorley]]

Comment on attachment 8626742 [details] [review]
PR 683

Almost there - looking good so far :-)

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8626742 [details] [review]
PR 683

And, 10 months later, I think this is done for real. :)

The first thing this PR does is make it so the page title won't display "undefined" temporarily as the page is loading (repoName takes a while to get populated, apparently). Instead, we'll just show "Treeherder" since that seems more accurate.

If we're looking at a single push ("revision" is a parameter in the URL), we try to parse the top commit message from that push.

We strip out things like multi-line commits (only take the first one), trychooser syntax, junk messages from mercurial queues, whitespace and punctuation. We do this for every commit in the push until we actually get something usable, at which point we break the loop. If none of the commits are usable (the only commit is trychooser syntax), we go back to just showing the repo name. We cut off the title at 70 characters so it doesn't get too long. 

We also return the completed percentage of jobs for the push so it can be included in the title.




There's a few ways the title will show depending on what we're looking at:

If we're looking at a single push and a usable commit message is found, the title will end up like this:
X% - [Y] repo: Commit

If we're looking at a single push and no usable commit message is found, the title will be:
X% - [Y] repo

If we're looking at more than a single push, the title will be:
[Y] repo


Where X is the percentage of jobs completed out of all jobs in the push, Y is the unstarred failure count for the entire view, "repo" is the name of the repository, and "Commit" is the parsed commit message.


I've toyed with the idea of changing out "100%" completed with the word "DONE" or something, but I guess that could go to a followup if that's desired. Don't know what everyone's preference for indicating a finished push would be. (Maybe we can color the favicon differently when viewing completed pushes or something?)

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8626742 [details] [review]
PR 683

Everything should have just worked the way it was, since `undefined` is falsey.

Changed that return value from `false` to `[false,false]` to make it less magical, though. :)

Comment 16

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/7b93f5d2030835db1895b60269e28947e181e8b2
Bug 1045602 - Port TBPL's feature to add a single revision's details to the page title (#683)

* Bug 1045602 - Port TBPL's feature to add a single revision's details to the page title r=emorley

Comment 17

[Ed Morley [:emorley]]

Wes I don't suppose you could file a bug for:

21:45 <philor> nice, the tab title for a DONTBUILD rev is NaN% - [0]

:-)

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Ed Morley [:emorley] from comment #17)
> Wes I don't suppose you could file a bug for:
> 
> 21:45 <philor> nice, the tab title for a DONTBUILD rev is NaN% - [0]
> 
> :-)

I actually couldn't reproduce that when I tested last night.

Comment 19

[Phil Ringnalda (:philor)]

Yeah, it's actually "Open a DONTBUILD rev; click 'Add new Jobs'" which is probably rare enough behavior to ignore.

Comment 20

[GitHub Autolander Bot]

Attachment: [treeherder] KWierso:followup1045602 > mozilla:master