Remove the "Revision URL List" feature since it doesn't display all changesets

RESOLVED FIXED

Status

Tree Management
Treeherder
P3
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: KWierso, Assigned: KWierso)

Tracking

Details

Attachments

(1 attachment)

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

Description

3 years ago
So there's a push to mozilla-central that lists 20 revisions, and also has the "... and 73 more" link that links to the full list of revisions for that push: https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-central&revision=b7eb1ce0237d

If I click the middle button for that push (to open the menu of other options like mcMerge, self-serve, and Revision URL List, then I click the Revision URL List option, a window pops up with the URLs to each revision in the push.

This is all great, but if you look in that pop up window, only the 20 revisions shown in Treeherder's UI are listed in the popup. The other 73 revisions are nowhere to be seen.

Bummer, man.

Updated

3 years ago
OS: Windows 8.1 → All
Priority: -- → P2
Hardware: x86_64 → All
Summary: Revision URL List window only lists the revisions visible on screen → Revision URL List window does not list revisions hidden under "... and N more"

Comment 1

3 years ago
Treeherder caps the number of revisions ingested at 200 as of bug 1119028, so there will be cases where we can't list all the revisions, even if we fix the UI here (though these will presumably not matter for the workflows you have in mind).

What's the specific use case for the revision URLs list vs say using mcMerge? :-)
Flags: needinfo?(wkocher)

Updated

3 years ago
Priority: P2 → P3
(Assignee)

Comment 2

3 years ago
I'd say the revision URL list could (should?) be dropped completely if it's not actually going to list all of the revisions.

My specific use case is to more easily select and copy the revision text to the clipboard for backouts, as opposed to having to try to alt-drag the linkified version in the main treeherder page.
Flags: needinfo?(wkocher)

Comment 3

3 years ago
Ah :-)

What about the list of commits in the console from the hg.m.o response?
(Assignee)

Comment 4

3 years ago
Lets say I needed to back out bug 1123920 from this push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f1fc4df52d3e
I didn't push it, so I don't have the hg.m.o response in my console.

I can use hg qbackout (or the mq-less hg oops that was recently added to the qbackout extension) to back out all three relevant revisions in a single backout commit with the -s flag. (hg oops -s -r <toprev>:<bottomrev>)

As it stands, I can get <toprev> (f1fc4df52d3e) from the push's header line by just double-clicking the revision. 

Getting <bottomrev> (b1f219eb3e57) is more involved, as I need to very carefully position the cursor at the end of the revision list and alt-dragging to the other side of it to select just the revision's text. If I move the mouse wrong, it selects all of the commit messages from the commits above or below the desired revision.

The revision URL list window makes this easier. I can just double-click the desired revision for each of those lines and copy them to the clipboard, then paste them into my console for the backouts

Comment 5

3 years ago
Oh you mean for generating the backout command - I thought you meant pasting the "sorry backed this bug out \n <URLs>".

In which case this seems like bug 1136918 is a better solution for you, yeah? (Fewer clicks).

In which case this bug really becomes "Fix or more likely remove the revision URL list feature".
Summary: Revision URL List window does not list revisions hidden under "... and N more" → Fix or remove the "Revision URL List" feature since it doesn't display all changesets
I still use the Revision List for marking bugs on uplifts in many cases because mcMerge still doesn't completely support all my use cases. I could work around not having it, but it would extra busy work to an already entirely-too manual process.

Comment 7

3 years ago
If it's useful, then lets fix it if it's not too much hassle :-)

That said it seems like making mcMerge support those workflows would save time in the long run. Could you file bugs for them if they're not already filed?
(Assignee)

Comment 8

3 years ago
Created attachment 8626442 [details] [review]
PR 677

Is this still important for you to have, Ryan?
Assignee: nobody → wkocher
Status: NEW → ASSIGNED
Flags: needinfo?(ryanvm)
Attachment #8626442 - Flags: review?(emorley)

Comment 9

3 years ago
Comment on attachment 8626442 [details] [review]
PR 677

Added a comment on the PR; more dead code to be removed.
Also depends on the other sheriffs confirming they are fine for this to be removed (ie that there aren't more bugherder tweaks needed first before all the use cases that the revision list covers, can be performed in bugherder).
Attachment #8626442 - Flags: review?(emorley) → feedback+
Per our offline discussion, I'm fine with this as long as hash selection is fixed in the main Treeherder UI.
Flags: needinfo?(ryanvm)
This is virtually ready to land, just needs some rebasing and a small tweak. Wes, do you want me to finish this up so we can close out the PR? :-) (We have quite a few open PRs at the moment)
Flags: needinfo?(wkocher)
(Assignee)

Comment 12

2 years ago
Comment on attachment 8626442 [details] [review]
PR 677

Made the changes and updated the PR.

Sorry this took so long to get back to.
Flags: needinfo?(wkocher)
Attachment #8626442 - Flags: review?(emorley)

Updated

2 years ago
Summary: Fix or remove the "Revision URL List" feature since it doesn't display all changesets → Remove the "Revision URL List" feature since it doesn't display all changesets
Comment on attachment 8626442 [details] [review]
PR 677

Almost there; but the current PR breaks the action button (test with web-server.js to see what I mean) - we need to keep the directive, but delete the link function, rather than delete all of the directive :-)
Attachment #8626442 - Flags: review?(emorley)
Attachment #8626442 - Flags: review-
Attachment #8626442 - Flags: feedback+

Comment 14

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

https://github.com/mozilla/treeherder/commit/c06ba00cd6c8e9eca9fe93554956bf132674d464
Bug 1112407 - Remove the revision list window

Since it doesn't display all changesets in the case of large pushes,
and its functionality is replaced by bugherder.
I've fixed this up, rebased and merged myself so we can close the PR and bug out :-)
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.