Page MenuHomePhabricator

Differential could provide actual content of context lines of code for inline comments
Closed, DuplicatePublic

Description

When doing an initial scan of the review comments it can sometimes be hard to figure out what the comment applies to. Arguably, I can always click on the line of code to see the comment in context, but it would be even better if I could see the code context in the list of the comments on top of the review. Many times there are only a couple of comments on the review that are narrowly scoped and providing the context would be extremely helpful in facilitating discussion. It's especially useful once the files are updated since you can no longer see the comments inline on the latest version.

Event Timeline

anton.vladimirov raised the priority of this task from to Needs Triage.
anton.vladimirov updated the task description. (Show Details)
anton.vladimirov added a subscriber: anton.vladimirov.

This would be especially useful in conjunction with T6874

chad added a subscriber: chad.Jan 6 2015, 1:23 AM

The main concern here is that type of UI is worse for as many people as it is better for. Our overall intention with Phabricator, specifically, is that it is a product built for improving code quality. For that, our expectations are that comments are well formed and thought out, not short quick snips. We also believe in information density, so the comments are a bit of a "table of contents" and that the discussion should occur directly on the code.

That said there are mocks floating around that add an additional dialog display (code + comments) that could be something you'd be interested in, but I don't see something like that being built for some time.

Here’s an example. There are 2 conversations and some one-offs going on here, but trying to follow what is being discussed is already fairly difficult. This is a mild example - it fit on one screen.

chad added a comment.Jan 6 2015, 9:06 PM

Our general recommendation is for people who unhappy with various aspects of Phabricator is that they should fork the project to meet their company's needs. Differential improvements / overhauls aren't on the current roadmap and suggestions are unlikely to thoughtfully addressed for years. We're simply too small (3 people) for spend as much time as the community sometimes wants debating fine details.

Here’s an example. There are 2 conversations and some one-offs going on here, but trying to follow what is being discussed is already fairly difficult. This is a mild example - it fit on one screen.

Doh! That was meant for T6876

That’s totally understandable. I was under the impression that you discourage local forks though. Seeing that all of my requests seem to converge onto T1460, if I wanted to help out with that how would I go about it?