Page MenuHomePhabricator

Add the repository name to revisions in Differential list view.
AbandonedPublic

Authored by 20after4 on Sep 22 2017, 9:04 PM.

Details

Reviewers
epriestley
chad
Group Reviewers
Blessed Reviewers
Summary

When you have a lot of projects it's not at all obvious which repo
a given revision belongs to.

Test Plan

Viewed phabricator.lan/differential, saw the repo listed under each revision.

Screenshot:

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 18526
Build 24951: Run Core Tests
Build 24950: arc lint + arc unit

Event Timeline

20after4 created this revision.Sep 22 2017, 9:04 PM
20after4 edited the test plan for this revision. (Show Details)Sep 23 2017, 7:55 AM
epriestley requested changes to this revision.Sep 23 2017, 2:17 PM

Sorry, not interested in bringing this upstream at this time. The code is fine if you want to patch it locally. See T4863 / T418 and linked tasks for some general discussion, I think. I think the current state of the world here is that this comes up every so often, but not meaningfully more than all the other "always show X on the list of Y" requests, and if we say "sure" to all of those requests the UI will be an unusuable mess of stuff that's only useful to 3% of users. T4863 / T418 talk about possibly supporting customization to deal with these cases.

This revision now requires changes to proceed.Sep 23 2017, 2:17 PM
20after4 abandoned this revision.Sep 26 2017, 9:16 AM

@epriestley I have no problem patching it locally (already have) it just seemed like an obvious thing to show on the differential screen. If it's not wanted upstream then it's no sweat off my back :)

I'm very much interested in this change too — when you have a bunch of diffs applying to different repos it's really really really useful to see it at a glance which repos they belong to.

So, question: would you be willing to consider a version of this diff that makes this behavior optional, defaulting to false (i.e., current behavior) ?

See T8227.

Well, the premise "It is usually much easier to get a change upstream if it does not add an option than if it does." doesn't seem true in this case, as you're opposed to accept the change without the optional behavior.

And, for what is worth, I totally agree with the general argument against options, but here you're opposing a proposed change that is really useful for people triaging diffs on a bunch of unrelated repositories.
I suspect that is why D9609 was accepted, but that solves the problem only for people using the email UI. The problem currently persists for people using the Web UI.