Git commit messages and PR etiquette

Transcript

1. Freitag, 23. Februar 2024 Git commit messages and PR etiquette Barbu Paul – Gheorghe barbu.paul.gheorghe@gmail.com
2. 2 wenglor sensoric group 2 About me • Programming since 2008 • Working since 2010 • Currently: Head of Software @ wenglor • Still, not an expert ¯\_( ツ )_/¯, raise your hand if you notice a mistake
3. 3 wenglor sensoric group 3 Prerequisites • Track issues (eg. Jira, Github) • Version code with git • A pull request (PR) system and do code reviews (eg. Bitbucket, Github, Gitlab)
4. 4 wenglor sensoric group 4 What is git & what are git commit messages
5. 5 wenglor sensoric group 5 https://github.com/torvalds/linux/commits/master?after=ac865f00af293d081356bec56eea90815094a60e+34&branch=master&qualified_name=refs%2Fheads%2Fmaster
6. 6 wenglor sensoric group 6 The goal: clean git history • Clean / clear messages (what & how) • Bigger feature – more commit messages, feature # from issue tracker • Small stuff – renames & so on, non-functional stuff – can have „symbolic messages“ • The history is the most important! • The reviewer of a PR should also inspect commit messages to be clean
7. 7 wenglor sensoric group 7 KO
8. 8 wenglor sensoric group 8 OK
9. 9 wenglor sensoric group 9 Better? https://www.conventionalcommits.org/en/v1.0.0/ <type>[optional scope]: <description> [optional body] [optional footer(s)] fix: prevent racing of requests Introduce a request id and a reference to latest request. Dismiss incoming responses other than from latest request. Remove timeouts which were used to mitigate the racing issue but are obsolete now. Reviewed-by: Z Refs: #123
10. 10 wenglor sensoric group 10 Git Hooks • pre-commit • prepare-commit-msg • commit-msg • post-commit • pre-push • post-checkout • post-merge • Demo: Enforce a Commit Template with backup for invalid messages • https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks • https://git-scm.com/book/en/v2/Customizing-Git-An-Example-Git-Enforced-Poli cy#_an_example_git_enforced_policy Server Side • pre-receive • update • post-receive • pre-rebase • post-rewrite Client Side
11. 11 wenglor sensoric group 11 prepare-commit-msg #!/bin/sh COMMIT_MSG_FILE=$1 COMMIT_SOURCE=$2 SHA1=$3 if [ "$COMMIT_SOURCE" == "message" ] # allow `git commit -m "..."` to use the user supplied message then echo "Commit source is "message"" exit 0 fi if [ "$(head -1 "$COMMIT_MSG_FILE")" != "" ] # allow `git commit --amend` to use the existing commit message (do not replace it) then echo "The commit message is not the default one" exit 0 fi MSG=$(cat <<EOM <type>[optional scope]: <description> [optional body] [optional footer(s)] EOM ) test -f commit-msg.backup && MSG=$(cat commit-msg.backup) echo "$MSG" > "$COMMIT_MSG_FILE"
12. 12 wenglor sensoric group 12 commit-msg #!/bin/sh test "" = "$(grep '<type>\|<description>' "$1")" || { cp "$1" commit-msg.backup echo >&2 Error, please commit again and write a commit type and a description exit 1 } # MSG=$(cat "$1") # MSG=${MSG//"[optional scope]"/} # MSG=${MSG//"[optional body]"/} # MSG=${MSG//"[optional footer(s)]"/} # echo "$MSG" > "$1" sed -i 's/\[optional scope\]//g;s/\[optional body\]//g;s/\[optional footer(s)\]//g' "$1" rm commit-msg.backup 2>/dev/null exit 0
13. 13 wenglor sensoric group 13 The goal: clean git history • Squash/rebase/amend – are just tools to get to a clean history • A PR can squash commits, but do it as part of the review process (not at the end) • This removes the need of asking „do we squash or not“ – this is the commiter‘s decision and also removes the need for a PR-task • ⚠⚠⚠ history manipulation should not be done on public/shared branches!
14. 14 wenglor sensoric group 14 If your changes are already public, all these commands have to be followed by a force push, talk to your colleagues!
15. 15 wenglor sensoric group 15 git commit --amend # edit the last commit’s message and/or files $ vim file.cpp $ git add file.cpp $ git commit -m “…“ $ vim file.h $ git add file.h # or other source code modifications $ git log --stat $ git commit --amend $ git log --stat ⚠⚠⚠ history manipulation should not be done on public/shared branches!
16. 16 wenglor sensoric group 16 git reset --soft # aka “squash” $ git reset --soft <sha> # squash all until this commit $ git commit … ⚠⚠⚠ history manipulation should not be done on public/shared branches!
17. 17 wenglor sensoric group 17 git rebase -i $ git rebase -i <sha> # squash and rename … ⚠⚠⚠ history manipulation should not be done on public/shared branches!
18. 18 wenglor sensoric group 18 Git Bisect • https://git-scm.com/docs/git-bisect • (debug-able history)
19. 19 wenglor sensoric group 19 Git Bisect Demo $ git bisect start $ git bisect bad $ git bisect good c33bc8 $ git bisect run ./autobisect.sh
20. 20 wenglor sensoric group 20 Git – Workflow, PRs, Code review • https://github.com/google/flatbuffers/pull/8157 https://nvie.com/posts/a-successful-git-branching-model/
21. 21 wenglor sensoric group 21 Responsibilities of a developer/reviewer – in the context of a good PR • Write/review code (seriously?) • Write/review tests • Write/review docs • Write/ensure good commit messages in order to have “debuggable” and useful history
22. 22 wenglor sensoric group 22