From e74797f55710b5a42917947b18fce0431d82a45d Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jun 2020 12:52:35 -0400 Subject: [PATCH] Start a contributor guide Addreses #122, #129 --- CONTRIBUTING.md | 195 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..f45cfb8 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,195 @@ +# How to Contribute + +Thank you for your interest in making ddclient better! This document +provides guidelines to make the contribution process as smooth as +possible. + +To contribute changes, please open a pull request against the +[ddclient GitHub project](https://github.com/ddclient/ddclient/pulls). + +## Style + + * Above all else, try to match the existing style surrounding your + edits. + * No trailing whitespace. + * Use spaces, not tabs. + * Indentation level is 4 spaces. + * Use parentheses for Perl function invocations: `print($fh "foo")` + not `print $fh "foo"` + * When reasonable, break lines longer than 99 characters. Rationale: + - Imposing a limit makes it practical to open many side-by-side + files or terminals without worrying about horizontal scrolling. + - 99 is used instead of 100 so that the +/- column added by + unified diff does not cause wrapping in 100 column wide + terminals. + * Add spaces to vertically align adjacent lines of code when doing + so improves readability. + +The following [perltidy](https://metacpan.org/pod/perltidy) command is +not perfect but it can get you close to our preferred style: + +```shell +perltidy -l=99 -conv -ci=4 -ola -ce -nbbc -kis -pt=2 -b ddclient +``` + +## Git Hygiene + + * Please keep your pull request commits rebased on top of master. + * Please use `git rebase -i` to make your commits easy to review: + - Put unrelated changes in separate commits + - Squash you fixup commits + * Write your commit message in imperative mood, and explain *why* + the change is made (unless obvious) in addition to *what* is + changed. + +If you are not very comfortable with Git, we encourage you to read +[Pro Git](https://git-scm.com/book) by Scott Chacon and Ben Straub +(freely available online). + +## Compatibility + +We strive to make ddclient usable on a wide variety of platforms. +Please limit yourself to Perl language features and modules available +on the following platforms: + + * Debian oldstable and newer + * Ubuntu, [all maintained + releases](https://ubuntu.com/about/release-cycle) + * Fedora, [all maintained + releases](https://fedoraproject.org/wiki/Fedora_Release_Life_Cycle) + * CentOS, [all maintained + releases](https://wiki.centos.org/About/Product) + * Red Hat Enterprise Linux, [all maintained + releases](https://access.redhat.com/support/policy/updates/errata/) + +See https://pkgs.org for available modules and versions. + +Exception: You may depend on modern language features or modules for +new functionality when no feasible alternative exists, as long as the +new dependency does not break existing functionality on old plaforms. + +All shell scripts should conform with [POSIX Issue 7 (2018 +edition)](https://pubs.opengroup.org/onlinepubs/9699919799/) or later. + +## Prefer Revert and Redo, Not Fix + +Suppose a recent change broke something or otherwise needs +refinement. It is tempting to simply push a fix, but it is usually +better to revert the original change then redo it: + + * There is less subjectivity with a revert, so you are more likely + to get a quick approval and merge. You can quickly "stop the + bleeding" while you and the project maintainers debate about the + best way to fix the problem with the original commit. + * It is easier and less mistake-prone to cherry-pick a single commit + (the redo commit) than two commits (the original commit plus the + required fix). + * Someone using blame to review the history will see the redo + commit, not the buggy original commit. + +## For ddclient Project Maintainers + +### Merging Pull Requests + +To facilitate reviews and code archaeology, `master` should have a +semi-linear commit history like this: + +``` +* f4e6e90 sandro.jaeckel@gmail.com 2020-05-31 07:29:51 +0200 (master) +|\ Merge pull request #142 from rhansen/config-line-format +| * 30180ed rhansen@rhansen.org 2020-05-30 13:09:38 -0400 +|/ Expand comment documenting config line format +* 01a746c rhansen@rhansen.org 2020-05-30 23:47:54 -0400 +|\ Merge pull request #138 from rhansen/dyndns-za-net +| * 08c2b6c rhansen@rhansen.org 2020-05-29 14:44:57 -0400 +|/ Replace dydns.za.net with dyndns.za.net +* d65805b rhansen@rhansen.org 2020-05-30 22:30:04 -0400 +|\ Merge pull request #140 from ddclient/fix-interpolation +| * babbef1 sandro.jaeckel@gmail.com 2020-05-30 04:03:44 +0200 +|/ Fix here doc interpolation +* 6ae69a1 rhansen@rhansen.org 2020-05-30 22:23:57 -0400 +|\ Merge pull request #141 from ddclient/show-debug-ssl +| * 096288e sandro.jaeckel@gmail.com 2020-05-30 04:42:27 +0200 +| | Expand tabs to spaces in vim +| * 0206262 sandro.jaeckel@gmail.com 2020-05-30 04:40:58 +0200 +|/ Show debug connection settings after evaluating use-ssl +... +``` + +See https://stackoverflow.com/a/15721436 for an explanation of the +benefits. + +This semi-linear style is mostly useful for multi-commit pull +requests. For single-commit pull requests, GitHub's "Squash and merge" +and "Rebase and merge" options are fine, though this approach still +has value: + + * The merge commit's commit message can link to the pull request + or contain other contextual information. + * It's easier to see who merged the PR (just look at the merge + commit author.) + * You can easily see both the original author timestamp (when the + change was made) and the merge timestamp (when it went live). + +To achieve a history like the above, the pull request must be rebased +onto `master` before merging. Unfortunately, GitHub does not have a +one-click way to do this (the "Rebase and merge" option does a +fast-forward merge, which is not what we want). See +[isaacs/github#1143](https://github.com/isaacs/github/issues/1143) and +[isaacs/github#1017](https://github.com/isaacs/github/issues/1017). Until +GitHub adds that feature, it has to be done manually: + +```shell +# Set this to the name of the GitHub user or project that owns the +# fork used for the pull request: +PR_USER= + +# Set this to the name of the branch in the fork used for the pull +# request: +PR_BRANCH= + +# The commands below assume that `origin` refers to the +# ddclient/ddclient repository +git remote set-url origin git@github.com:ddclient/ddclient.git + +# Add a remote for the fork used in the PR +git remote add "${PR_USER:?}" git@github.com:"${PR_USER:?}"/ddclient + +# Fetch the latest commits for the PR and ddclient master +git remote update -p + +# Switch to the pull request branch +git checkout -b "${PR_USER:?}-${PR_BRANCH:?}" "${PR_USER:?}/${PR_BRANCH:?}" + +# Rebase the commits (optionally using -i to clean up history) onto +# the current ddclient master branch +git rebase origin/master + +# Force update the contributor's fork. This will only work if the +# contributor has checked the "Allow edits by maintainers" box in the +# PR. If not, you will have to manually merge the rebased commits. +git push -f + +# If the force push was successful, you can now go into the GitHub UI +# and merge using the "Create a merge request" option. +# +# If the force push failed because the contributor did not check +# "Allow edits by maintainers", or if you prefer to merge manually, +# continue with the next steps. + +# Switch to the local master branch +git checkout master + +# Make sure the local master branch is up to date +git merge --ff-only origin/master + +# Merge in the rebased pull request branch **WITHOUT DOING A +# FAST-FORWARD MERGE** +git merge --no-ff "${PR_USER:?}-${PR_BRANCH:?}" + +# Review the commits before pushing +git log --graph --oneline --decorate origin/master.. + +# Push to ddclient master +git push origin master +```