Page MenuHomePhabricator

Add commenting to differential previews
Closed, DuplicatePublic

Description

I occasionally create preview diffs to get feedback on proposed changes from colleagues before I'm ready to say with confidence that the changes are ready to be committed to our codebase. It would be helpful if those colleagues were able to add line and overall comments to the diff.

Thank you!

Event Timeline

bradley created this task.Sep 17 2015, 5:38 PM
bradley updated the task description. (Show Details)
bradley added a project: Differential.
bradley added a subscriber: bradley.

Are you using Differential to create revisions of changes prior to moving commits upstream? What you describe is essentially the workflow that using Differential is for, as far as I understand.

chad added a subscriber: chad.Sep 17 2015, 5:47 PM

Yeah, it's not clear what you're currently doing that does not have commenting. Differential has commenting, specifically for pre-commit review.

The workflow I was using was arc diff --preview, followed by sending a link to the diff to the colleague I wanted feedback from. In my org, creating a revision without preview is equivalent to saying "I feel confident that this code is ready to be committed to master," which isn't always the case.

avivey added a subscriber: avivey.Sep 17 2015, 5:57 PM

Your org has an... unusual workflow.
Revision is the the tool that is designed for pre-commit reviews; It supports all the things that you'd want from such a tool, and based on the assumption that creating a revision is not saying "this is ready to be committed", but "This is ready to be reviewed".

chad added a comment.Sep 17 2015, 5:57 PM

I just put [WIP] or [DRAFT} in my diff title to make this clear to colleagues that the diff is rough, but early feedback is wanted.

The biggest value of Differential (pre-commit) are architectural or high level discussions. These discussions should happen in Differential, with the correct audience. This facilitates the right course of action, with documentation and somewhere to reference in the future (as opposed to whiteboards or emails). If you're only submitting "production ready" into Differential, you're missing some of the main benefits.

You can also create revisions without attaching reviewers/subscribers -- this is what we do when we have commits which we also plan for more changes prior to review. Revisions also have a status of "Changes Planned" or something similar.