2

I am trying to teach good engineering practices to my team, and in particular, walking them through the basics of PRs and code reviews on GitHub. One of my team members has sent me a request to review his PR, but has added a commit message without any detail, and so I would like to kick it back to him for a better commit message. There doesn't seem to be a way to do this directly in GitHub. I could just add a "Request Changes" with a general comment on the commit message and ask him to fix it, but how should I tell him to do this? I know it's possible to do this as a git commit --amend followed by a git push --force, but the docs also say that this is really bad practice for a commit that's already been pushed, and I don't want to teach any bad habits. What's the best way to fix this?

Improve this question
3
  • 1
    Why does it matter? Why not just squash the commits and add a single unifying message when merging the pull request?
    – Thomas Owens ♦
    Commented Aug 9, 2019 at 0:10
  • 6
    Rewriting the commit while it's still on an unmerged branch is OK. The issues come when you rewrite commits that other people have started to base their own work on.
    – jonrsharpe
    Commented Aug 9, 2019 at 6:45
  • It's a bad habit for a commit pushed to a shared branch. That doesn't apply to a branch being reviewed before merge; it's after merge that rewrites are verboten.
    – Charles Duffy
    Commented Mar 18, 2023 at 14:56

1 Answer 1

Reset to default
2

You are correct, there is no way to change the commit message without redoing the commit and presumably the branch has now been shared for code review purposes.

You could get them to redo the work in a new branch and do a new PR on that, but it sounds super unproductive.

Presumably as the team is new to the process, mistakes will be made. You are just going to have to let this one slide.

Also, although commit messages are useful, really you shouldn't be relying on them for documentation. I would not include them in code review. Ask the dev to add comments to the code, update the documentation, link to the ticket etc etc

Improve this answer
1
  • 3
    Commits do make good docs though—git blame is a part of my toolbox, and its very frustrating not to be able to use it to eg figure out if a strange line of code is fixing an obscure bug or introducing it.
    – D. Ben Knoble
    Commented Aug 9, 2019 at 21:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.