From 4c62e17cc008bdaf8a50332ed911ccade51bf7a6 Mon Sep 17 00:00:00 2001 From: David Thompson Date: Wed, 24 Jun 2015 16:01:26 -0400 Subject: Add "Happy Patching". --- happy-patching/happy-patching.org | 168 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 happy-patching/happy-patching.org (limited to 'happy-patching/happy-patching.org') diff --git a/happy-patching/happy-patching.org b/happy-patching/happy-patching.org new file mode 100644 index 0000000..2888bc6 --- /dev/null +++ b/happy-patching/happy-patching.org @@ -0,0 +1,168 @@ +#+TITLE: Happy Patching +#+AUTHOR: David Thompson +#+EMAIL: dthompson@vistahigherlearning.com +#+DATE: 2015-06-25 +#+DESCRIPTION: +#+KEYWORDS: +#+LANGUAGE: en +#+OPTIONS: H:3 num:t toc:t \n:nil @:t ::t |:t ^:t -:t f:t *:t <:t +#+OPTIONS: TeX:t LaTeX:t skip:nil d:nil todo:t pri:nil tags:not-in-toc +#+INFOJS_OPT: view:nil toc:nil ltoc:t mouse:underline buttons:0 path:http://orgmode.org/org-info.js +#+EXPORT_SELECT_TAGS: export +#+EXPORT_EXCLUDE_TAGS: noexport +#+LINK_UP: +#+LINK_HOME: +#+startup: beamer +#+LaTeX_CLASS: beamer +#+LaTeX_CLASS_OPTIONS: [bigger] + +* Introduction + +*** Goals + + - Improve the commit log + - Improve the quality of individual patches + - Improve the quality of pull requests + - Improve the code review workflow + +*** Why? + + - More readable history + - Easier to understand *why* a change was made + - Easier to =git bisect= to find breaking changes + - Easier to =git revert= those breaking changes + +* Commits + +** Guidelines + +*** What's in a patch? + + A patch: + + - Stands alone as a single, complete, logical change + - Has a descriptive change log message + - Has no extraneous modifications (whitespace changes, fixing a + typo in an unrelated file, etc.) + - Follows established coding conventions closely + +*** Example :B_example: + :PROPERTIES: + :BEAMER_env: example + :BEAMER_opt: shrink=5 + :END: + + #+BEGIN_SRC diff + aad72327d17a1479f586af3cdb7123ffec2d9719 + Author: Ricardo Wurmus + Date: Tue Jun 23 16:35:16 2015 +0200 + + view: json: Add "location" field to JSON representation. + + ,* guix/web/view/json.scm (package->json): Add "location" field. + + 1 file changed, 4 insertions(+) + guix/web/view/json.scm | 4 ++++ + + Modified guix/web/view/json.scm + diff --git a/guix/web/view/json.scm b/guix/web/view/json.scm + index e3f8bc1..73b78f3 100644 + --- a/guix/web/view/json.scm + +++ b/guix/web/view/json.scm + @@ -24,8 +24,9 @@ + #:use-module (web uri) + #:use-module (guix licenses) + #:use-module (guix packages) + #:use-module (guix profiles) + + #:use-module (guix utils) + #:use-module (gnu packages) + #:use-module (guix web package) + #:export (all-packages-json + view-package-json + @@ -62,8 +63,11 @@ + ("name" ,(package-name package)) + ("version" ,(package-version package)) + ("synopsis" ,(package-synopsis package)) + ("description" ,(package-description package)) + + ("location" ,(last (string-split (location-file + + (package-location package)) + + #\/))) + ("homepage" ,(package-home-page package)) + ("license" ,(serialize-license package)) + ,@(if serialize-inputs? + `(("inputs" ,(serialize-inputs (package-inputs package))) + #+END_SRC + +** Short Log + +*** Short Log + + The first line of a commit log should: + + - Be a short sentence (\leq 72 characters maximum, but shoot for + \leq 50) + - Use imperative, passive voice ("Add awesome feature." vs. "Added + awesome feature.") + - Prefix with an identifier for the general area you were working + 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) + - Be word wrapped to 72 characters per line + +* Pull Requests + +** Guidelines + +*** Pull Requests + + A pull request should: + + - Have a descriptive title and summary of the changes made + - Contain separate commits for logically separate changes + - 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) + +** Workflow + +*** Programmer Workflow + + - Commit as often as you'd like, but squash or otherwise rewrite + your commits into logical patches before asking for code review + - Consider WIP branches ("story\textunderscore{}XXXX", + "task\textunderscore{}XXXX", etc.) to be volatile (because they + are), and anticipate that they could be rebased at any moment + - In response to feedback, squash the new "fix up" commits into + the respective commit that is being fixed with an interactive + rebase + - Push the new, rewritten branch with a =git push --force= (Scary! + But GitHub doesn't play nicely with a safer method) + +*** Reviewer Workflow + + - Inspect patches individually in addition 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 + +* References + +*** References + + - Git Patch Guidelines — + http://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches?id=HEAD + - GNU Change Log Standards — + https://www.gnu.org/prep/standards/html_node/Change-Logs.html + - On Code Review — + http://glen.nu/ramblings/oncodereview.php -- cgit v1.2.3