Skip to content

Latest commit

 

History

History
294 lines (227 loc) · 10.6 KB

CONTRIBUTING.md

File metadata and controls

294 lines (227 loc) · 10.6 KB
Raw

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.

Developer Certificate of Origin

All contributions are subject to the Developer Certificate of Origin v1.1, copied below. A Signed-off-by line in each commit message is not required.

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

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 command is not perfect but it can get you close to our preferred style:

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 your 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 by Scott Chacon and Ben Straub (freely available online).

Unit tests

Always add tests for your changes when feasible.

To run the ddclient test suite:

  1. Install GNU Autoconf and Automake
  2. Run: ./autogen && ./configure && make VERBOSE=1 check

To add a new test script:

  1. Create a new t/*.pl file with contents like this:

    use Test::More;
# Your test dependencies go here.

SKIP: { eval { require Test::Warnings; } or skip($@, 1); }
eval { require 'ddclient'; } or BAIL_OUT($@);

# Your tests go here.

done_testing();

    See the documentation for Test::More for details.

  2. Add your script to the handwritten_tests variable in Makefile.am.

  3. If your test script requires 3rd party modules, add the modules to the list of test modules in configure.ac and re-run ./autogen && ./configure. Be sure to skip the tests if the module is not available. For example:

    eval { require Foo::Bar; } or plan(skip_all => $@);

Compatibility

We strive to find the right balance between features, code maintainability, and broad platform support. To that end, please limit yourself to Perl language features and modules available on the following platforms:

See https://pkgs.org for available modules and versions.

Exceptions:

  • 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.
  • Test scripts may depend on arbitrary modules as long as the tests are skipped if the modules are not available. Effort should be taken to only use modules that are broadly available.

You may use any core Perl module as long as it is available in all versions of Perl we support. (Though please make sure it is listed in the appropriate configure.ac check.) Stated another way: We are not interested in supporting platforms that lack some core Perl modules, unless doing so is trivial.

All shell scripts should conform with POSIX Issue 7 (2018 edition) 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 and isaacs/github#1017. Until GitHub adds that feature, it has to be done manually:

# 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