Page MenuHomePhabricator

Add support to comment on commit message text
Closed, WontfixPublic

Description

Hi,

It would be nice if Differential could support comments in the "Local Commits" table. Things like "your commit message is unclear or confusing", "major typo" etc. We are using the immutable workflow with git and want to enforce clean and clear commit messages as well. So this should be part of the review as well.

Thanks!
reto

Event Timeline

reet- raised the priority of this task from to Needs Triage.
reet- updated the task description. (Show Details)
reet- added a project: Differential.
reet- added a subscriber: reet-.
epriestley claimed this task.

I don't think this feature would be useful enough to enough users to justify the implementation complexity. Particularly, no one has asked for anything similar in the past and feedback we have received tends to focus more on hiding or reducing the prominence of this element.

Using normal text also seems like a pretty reasonable workaround ("Your commit message on xxx isn't very clear."). With a truly immutable workflow, these errors also can't be fixed?

I'll keep an ear out for similar reports or use cases, but even the UI for doing this would necessarily add a lot of complexity and I'd want to see considerable additional interest in this feature before we considered building it.

Philosophically, we also favor a mutable, one-idea-is-one-commit workflow (see https://secure.phabricator.com/book/phabflavor/article/recommendations_on_revision_control/). An advantage this approach has is that commit messages strongly tend to be clear and expressive (see Phabricator's history for an example). Presumably you've made a reasoned decision to use the workflow you prefer, but if you haven't considered a mutable, one-to-one workflow, it might be worth looking at.

Thanks for your feedback.

Using normal text is indeed a reasonable workaround if no one else wants such a feature.

We allow squash/rebase operations in feature branches (devel-myfeature) until the feature is landed (onto devel branch), so one can fix unclear git messages or typos as well.

We were considering a mutable workflow but prefer to have one-reviewed-merge-commit consisting of >= 1 commits is one feature/idea (see muen devel branch). We want to preserve the iterative commit history with small commits. I also prefer to checkout bigger feature branches and review each commit separately with tig and then use Differential to give feedback. We are new to Phabricator (great stuff btw!) and might adopt our workflow over time.