From 2a48e3161f1469cc830ec3dbe336e1fc8f0418b1 Mon Sep 17 00:00:00 2001 From: David Thompson Date: Thu, 25 Jun 2015 10:16:05 -0400 Subject: Expand "Happy Patching". --- happy-patching/happy-patching.org | 36 +++++++++++++++++++++++------------- happy-patching/happy-patching.pdf | Bin 101542 -> 105315 bytes 2 files changed, 23 insertions(+), 13 deletions(-) (limited to 'happy-patching') diff --git a/happy-patching/happy-patching.org b/happy-patching/happy-patching.org index 2888bc6..f5370e3 100644 --- a/happy-patching/happy-patching.org +++ b/happy-patching/happy-patching.org @@ -31,11 +31,11 @@ - Easier to understand *why* a change was made - Easier to =git bisect= to find breaking changes - Easier to =git revert= those breaking changes + - Easier to review pull requests + - Faster code review loop * Commits -** Guidelines - *** What's in a patch? A patch: @@ -93,8 +93,6 @@ `(("inputs" ,(serialize-inputs (package-inputs package))) #+END_SRC -** Short Log - *** Short Log The first line of a commit log should: @@ -107,21 +105,25 @@ in ("tests: Fix the frob." or "gradebook: Give everyone an A.") - Always end with a period. -** Log Body - *** Log Body The body of a commit log should: - Explain or justify the change - For a bug fix, provide a ticket number or link to the ticket - - Explain what changes were made at a high level (see: The GNU - ChangeLog standard) + - Explain what changes were made at a high level (The GNU ChangeLog + standard is worth a read) - Be word wrapped to 72 characters per line -* Pull Requests +*** Workflow -** Guidelines + - Review the full diff before commiting (don't =git add= and + immediately =git commit=) + - Use before commit hooks to run linters such as Rubocop + - Use your =$EDITOR=, not the =-m= flag, for writing your commit + log + +* Pull Requests *** Pull Requests @@ -132,9 +134,15 @@ - Not contain *any* "fix up" commits ("Fix typo.", "Fix test.", "Remove commented code.") - Be able to be thoroughly reviewed by a single person (No massive - patch sets containing weeks of work by multiple people) + patch sets containing weeks of work by several people) + +*** Code Review Goals -** Workflow + - Shared responsibility between submitter and reviewer + - Prioritize code review + - Disassociate pull requests from being strictly tied to a + story/epic/task/etc. + - Make code review → QA → production phases happen faster *** Programmer Workflow @@ -151,7 +159,7 @@ *** Reviewer Workflow - - Inspect patches individually in addition to looking at the full + - Inspect patches individually as opposed to looking at the full diff in GitHub's web interface; each commit should stand alone - Refer to coding conventions when pointing out style problems - Follow up on changes made in response to your feedback quickly @@ -166,3 +174,5 @@ https://www.gnu.org/prep/standards/html_node/Change-Logs.html - On Code Review — http://glen.nu/ramblings/oncodereview.php + - A Note About Git Commit Messages — + http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html diff --git a/happy-patching/happy-patching.pdf b/happy-patching/happy-patching.pdf index ab3a76d..920da7a 100644 Binary files a/happy-patching/happy-patching.pdf and b/happy-patching/happy-patching.pdf differ -- cgit v1.2.3