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

RESOLVED FIXED

Status

Tree Management
Treeherder
P4
normal
RESOLVED FIXED
4 years ago
10 months ago

People

(Reporter: Paolo, Assigned: KWierso)

Tracking

({regression})

Details

Attachments

(1 attachment, 1 obsolete attachment)

46 bytes, text/x-github-pull-request
emorley
: review+
emorley
: feedback+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
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

4 years ago
Is the solution used by TBPL's recent bug 1020919 something that sounds reasonable?
Blocks: 1030636
Priority: -- → P3
(Reporter)

Comment 2

4 years ago
(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

4 years ago
(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).
Summary: The page title for individual revisions should include details → The page title for individual revisions should include details (port bug 1020919 to treeherder)

Updated

3 years ago
Priority: P3 → P2

Updated

3 years ago
Blocks: 1059400
No longer blocks: 1030636

Updated

3 years ago
Priority: P2 → P3

Updated

3 years ago
Keywords: regression

Updated

3 years ago
Priority: P3 → P4

Updated

3 years ago
No longer blocks: 1059400

Updated

3 years ago
Duplicate of this bug: 1171376
(Assignee)

Comment 6

3 years ago
Created attachment 8626742 [details] [review]
PR 683
Assignee: nobody → wkocher
Status: NEW → ASSIGNED
Attachment #8626742 - Flags: review?(emorley)

Comment 7

3 years ago
Comment on attachment 8626742 [details] [review]
PR 683

Thank you for looking at this :-)
Attachment #8626742 - Flags: review?(emorley) → feedback+
(Assignee)

Comment 8

3 years ago
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).)
Attachment #8626742 - Flags: review?(emorley)

Comment 9

3 years ago
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 :-)
(Assignee)

Updated

3 years ago
Blocks: 1188027
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1188027
(Assignee)

Comment 11

3 years ago
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 on attachment 8626742 [details] [review]
PR 683

Almost there - looking good so far :-)
Attachment #8626742 - Flags: review?(emorley) → review-

Updated

2 years ago
Duplicate of this bug: 1209197
(Assignee)

Comment 14

2 years ago
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?)
Attachment #8626742 - Flags: review- → review?(emorley)

Updated

2 years ago
Attachment #8626742 - Flags: review?(emorley)
(Assignee)

Comment 15

2 years ago
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. :)
Attachment #8626742 - Flags: review?(emorley)

Updated

2 years ago
Attachment #8626742 - Flags: review?(emorley) → review+

Comment 16

2 years ago
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
(Assignee)

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
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]

:-)
(Assignee)

Comment 18

2 years ago
(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.
Yeah, it's actually "Open a DONTBUILD rev; click 'Add new Jobs'" which is probably rare enough behavior to ignore.
(Assignee)

Updated

2 years ago
Blocks: 1279359
Comment hidden (typo)

Updated

2 years ago
No longer blocks: 1279359
Depends on: 1279359

Updated

2 years ago
Attachment #8761829 - Attachment is obsolete: true

Updated

10 months ago
Depends on: 1348276
You need to log in before you can comment on or make changes to this bug.