OSSDev: Advanced Git Mind your Git Manners Irina Gulina Senior Quality Engineer Tomas Tomecek Principal Software Engineer Red Hat Happy Birthday, Git! Source: https://octodex.github.com/ Agenda • Commit • Push • PR/MR submitting and review • Merge • Rebase • Quiz 3 4^ Red Hat Commit content • Don't: Two and more changes in one commit 1e4f aaO Fix login timeout BZ, add logout step • Do: One commit = One logical change 1e4faaO Fix login timeout BZ 2r5asy8 Add logout step Commit content • Separate whitespace changes from code changes, especially unrelated. o Mixing those is a great way to introduce a bug and o Complicates code review 6 4^ Red Hat What is a commit message? • Title/subject line • Body Commit message example $ git log Commit Title or Subject line commit Author: / Commit Body Date: Mon Apr 2 15:10:03 2020-0400 ' Change how workers are represented * Don't serialize the 'gracefully_shutdown' field * Create a new 'missing' property and serialize it * In the status API, list both online and missing workers Requires PR: https://github.com//pull/921 closes #354498 https://bugzilla. redhat.com/show_bug. cgi?id=354498 8 4^ Red Hat What is a "bad" message? $ git log — -oneline -5 --author irina —before "Wed Apr 7 2021" dcc2o!35 address comments b7aac30 fix issue #123 0b7a4e4 various docs fixes 1e4faa0 ui bug fix fc3d081 readme update d21660dc ToDo 0b7a4e4 Mix fixes and cleanups 5h3d28g refactoring 9 4^ Red Hat What is a "bad" message? $ git log — -oneline -5 --author irina —before "Wed Apr 7 2021" dcc2o!35 address comments <- what comments? b7aac30 fix issue #123 <- of what project? 0b7a4e4 various docs fixes <- what docs? why? 1e4faa0 ui bug fix <- what was the bug? fc3d081 readme update <- why? d21660dc ToDo <- @©@© 0b7a4e4 Mix fixes and cleanups <-AA ft 5h3d28g refactoring <-© Uninformative, look-elsewhere commit messages (titles) 4^ Red Hat Usage of a commit title • git log —pretty=oneline • git rebase —interactive • merge.summary • gitshortlog • git format-patch, git send-email,... • reflogs • GUI tools for committing and browsing • GitHub, SourceForge, Bitbucket, GitLab,... service 11 4^ Red Hat Poor quality code can be refactored A terrible commit message lasts forever. 12 Red Hat For whom do you write commit messages? 13 Red Hat Why should I write 'good' commit messages? • To help to understand the code change o What has been changed? o Why is that change necessary? • To speed up the reviewing process • To help to locate a bug • To write a good release note or script it 14 4^ Red Hat What constitutes a good commit message? • git commit -m "Fix login timeout bug" • git commit or git commit —verbose Redirect user to the requested page after login https://link/to/issue/tracker 15 4^ Red Hat What constitutes a good commit message? • Capital letter, SO/72, no punctuation in the end $ git commit A brief summary of the commit A paragraph describing what changed and its impact. What constitutes a good commit message? • Present Tense and Imperative Mood cf31dl2 Adds login unit tests 7a9kj4f Fixed login unit tests 101q2wd Update login unit tests Ib7hn61 Removing login unit test "If accepted, this commit will ." 17 4^ Red Hat Ticket number in commit messages • Ticketing system != git log o "TICKET-123456 add missing params to class" o "Add missing meta fields to response" □ Takes space in 50 chars limit title □ Look-elsewhere for details message, I'm lazy □ May be not available for interested user or reviewer (permissions, outage) What constitutes a good commit message? • Clear Title- What is commit about? • Present Tense and Imperative Mood • No punctuation in a title • Clear Body - What and why is it needed/changed vs how? • 50/72 • Reference to an issue in a body message • Follow the commit convention defined by the team 19 4^ Red Hat 20 4t Red Hat IF YOU DO FORCE PUSH... May the force stay with you. 21 Red Hat Git push —force trap • It's ok to force push to your local branch • It's ok to force push to your (unmerged/open) PR/MR • It's not ok to force push to a public branch 22 4^ Red Hat Git push —force consequences Git push —force consequences • Lost data • Altered history • Not happy colleagues • Lost karma points How to avoid unwanted force push How to avoid unwanted force push • Protect important branches • Backup • Use git checkout-b • Use —force-with-lease, carefully • Use PR revert You have a great freedom... to change your history locally. 27 Red Hat Submitting a PR Red Hat Why do we use PR/MR workflow? Why do we use PR/MR workflow? • Share changes • Get review and feedback • Encourage quality What constitutes a good PR/MR? What constitutes a good PR/MR? • Complete piece of work • Adds value in some way • Solid title and body • Clear commit history • Small • Meets project's contribution guidelines Contributors (before submitting a • Follow the repo's conventions • Double check your code (and ToDos) • Add docs • Keep changes small • Separate branch • Be clear and specific • Check your ego and be polite, Open Source Contribution (GitHub) Providerless tag for client go auth plugins #100606 k8s-ci-robot merged 1 commit into kubemetesmaster from dijB:pravid ♦ inkscape > Merge Requests 12946 Merged Created 4 days ago by Viil Jyoti Balodhi Contributor Typo Fixed parent ->parentltem Overview 6 Commits 1 Pipelines 3 Changes 1 All threads resolved Hello, I have fixed a typo in https://gitlab.eom/inkscape/inkscape/-/blob/master/src/selection-chemistry.cpp#L3007-3009 as:- SPItem 'parentltem = dynamic cast(parent); if (parentltem) { // parent was being checked here before rather than parentltem parent transform = parentItem->i2doc affine(); Edited 4 days ago by Jyoti Balodhi Request to merge Jyoti Balodhi: fix fj, into master (2) Merged result pipeline #277765637 passed for 48600ee5 0© A v fl'' Merge request approved. Approved by ^ > $ 0 0 O view e|igib|e approvers 35 (v) Merged by $ Thomas Holder 1 day ago The changes were merged into master with cbfa6879 (J The source branch has been deleted Red Hat Contributors (after submitting a PR/MR) • Check your ego and be polite o @username ping! o @username review please • Ensure your branch merge and tests pass • Use —amend, —fixup or rebase -i • Don't merge your own PR WIP PR/MR • Don't overuse WIP label • Remove WIP label when ready • "This is ready for review, please." Reviewing a PR 38 4t Red Hat PR Reviewers • Be kind and polite o @username ping, error here! o @username s/foo/bar • Check commit history • Don't fix issues • Ensure the branch can be merged • CI Tests pass • Don't merge WIPs • Squash • Delete branch 40 Source: https://octodex.github.com/ Red Hat 41 4t Red Hat 42 0 Brand New Commits Source: https://www.atlassian.com/qit/tutorials/rewritinq-historv/qit-rebase 4fc Red Hat Interactive rebase 1 r c11d286 init: improve UX: 2 squash bfc9e39 init: default CentOS dist-git branch to c9s 3 fixup 4e5599b refactor hostname strings into constants 4 edit C870153 SourceGitGenerator: run %prep if spec has applied patches pick 08b4efd init: enable using Stream 9 dist-git as a source P 7 # Rebase c75edaf..08b4efd onto c75edaf (5 commands) 8 # 9 # Commands: 10 # p, pick = use commit 11 # r, reword = use commit, but edit the commit message 12 # e, edit = use commit, but stop for amending 13 # s, squash = use commit, but meld into previous commit 14 # f, fixup = like "squash", but discard this commit's log message 15 # x, exec = run command (the rest of the line) using shell 16 # b, break = stop here (continue rebase later with 'git rebase --continue') 17 # d, drop = remove commit 18 # 1, label