The first line of a commit message should be < 80 characters long and briefly describe the whole commit. Optionally, you can prefix the summary with a tag/module in square brackets (e.g. travis if your commit changed something for Travis, or testsuite for the testsuite, or tiff-parser, etc.). If the commit requires additional explanation, a blank line can be put below the summary followed by a more thorough explanation.
A commit message can look like this:
[travis] Fix mac osx jobs
- Specify concrete ubuntu and mac versions
- Use latest conan version
- Fix the profiles for linux and mac
- Use new version of expat (avilable in conan-center)
- Install urllib3 as suggested in python guidelines
- Use virtualenv with python3
The advantage of this approach is that we always see the brief
summary via git log --oneline
and on GitHub. The 80
characters limit ensures that the message does not wrap.
Please avoid overly generic commit messages like "fixed a bug", instead write e.g. "fixed an overflow in the TIFF parser". If your commit fixes a specific issue on GitHub then provide its number in the commit message. A message of the form "fixes #aNumber" result in GitHub automatically closing issue #aNumber once the issue got merged (please write that in the detailed description below the summary). If the commit fixes an issue that got a CVE assigned, then you must mention the CVE number in the commit message. Please also mention it in commit messages for accompanying commits (like adding regression tests), so that downstream package maintainers can cherry-pick the respective commits easily.
If you have trouble finding a brief summary that fits into 80 characters, then you should probably split your commit.
Commits should be atomic, i.e. they should make one self-contained change. Consider the following example: you want to fix an issue, which requires you to perform two changes in two separate files to fix the issue. Then you also want to reformat both files using clang-format and add a regression test or a unit test.
This would result in the following commits:
We can summarize this in the following guidelines:
Large formatting changes should be separate commits for each source file (that way changes can be reviewed easily, as formatting changes result in very noisy diffs)
Changes made in different source which do not make sense without each other should be committed together
Changes made in the same file which do not belong together should not be committed together
If changes are requested during the code review, then they should be either included in the previous already created commits, if that is applicable. For example if a variable's name should be changed, then that should be included into the already created commit. A bigger change, like a new function or class will probably require a separate commit.
Please keep in mind that your commits might be cherry-picked into an older branch. Therefore split your commits accordingly, so that changes into separate modules go into separate commits.
Every commit should keep the code base in a buildable state. The
test suite needn't pass on every commit, but must pass before being
merged into master
.
These are however not strict rules and it always depends on the case. If in doubt: ask.
We prefer to keep the git log nearly linear with the individual pull requests still visible, since they usually form one logical unit. It should look roughly like this:
* 9f74f247 Merge pull request #227 from frli8848/master
|\
| * 73ac02d7 Added test for Sigma lenses
| * fc8b45dd Added the Sigma 120-300mm F2.8 DG OS HSM | S for Nikon mount.
| * 34a3be02 Added Sigma 50mm F1.4 DG HSM | A mount/UPC code (for Nikon mount).
| * 21522702 Added Sigma 20mm F1.4 DG HSM | A mount/UPC code (for Nikon mount).
|/
* f9d421b1 Merge pull request #109 from D4N/error_codes_enum
|\
| * 3965a44d Replace error variable names in test suite with enum error codes
| * a15f090f Modified test suite so that case sensitive keys are possible
| * efe2ccdc Replaced all hardcoded error codes with ker... constants
| * d897997b Force error code usage to construct a Exiv2::BasicError
| * d3c3c036 Incorporated error codes into errList
| * b80fa1b4 Added error codes from src/error.cpp into an enumeration
|/
* efee9a2b Merge pull request #205 from D4N/CVE-2017-1000127_reproducer
As can be seen, the two pull requests are still distinguishable but the history is still nearly linear. This ensures that cherry-picking and bisecting works without issues.
To ensure such a linear history, do not use GitHub's
Update Branch
button! This creates a merge commit in your
pull request's branch and can results in rather complicated logs, like
this:
* |
|\ \
| * |
* | |
|\ \ \
| |/ /
|/| |
| * |
| * |
| * |
| * |
| * |
|/ /
* |
|\ \
| |/
|/|
| *
| *
| *
|/
*
Instead of using the Update Branch
button use
git pull --rebase
. For the following example, we'll assume
that we are working in a branch called feature_xyz
that
should be merged into the branch master
. Furthermore the
remote origin
is a fork of exiv2 and the remote
upstream
is the "official" exiv2 repository.
Before we start working, the master
branch looks like
this:
$ git log master --oneline --graph
* efee9a2b (master) Merge pull request #something
|\
| * ead7f309 A commit on master
|/
* 55001c8d Merge pull request #something else
We create a new branch feature_xyz
based on
master
, create two new commits My commit 1
and
My commit 2
and submit a pull request into master. The log
of the branch feature_xyz
now looks like this:
$ git log feature_xyz --oneline --graph
* 893fffa5 (HEAD -> feature_xyz) My commit 2
* a2a22fb9 My commit 1
* efee9a2b (master) Merge pull request #something
|\
| * ead7f309 A commit on master
|/
* 55001c8d Merge pull request #something else
If now new commits are pushed to master
, resulting in
this log:
$ git log master --oneline --graph
* 0d636cc9 (HEAD -> master) Hotfix for issue #something completely different
* efee9a2b Merge pull request #something
|\
| * ead7f309 A commit on master
|/
* 55001c8d Merge pull request #something else
then the branch feature_xyz
is out of date with
master
, because it lacks the commit 0d636cc9
.
We could now merge both branches (via the cli or GitHub's
Update Branch
button), but that will result in a messy
history. Thus don't do it! If you do it, you'll have to
remove the merge commits manually.
Instead run: git pull --rebase upstream master
in the
feature_xyz
branch. Git will pull the new commit
0d636cc9
from master into your branch
feature_xyz
and apply the two commits
My commit 1
and My commit 2
on top of it:
$ git log feature_xyz --oneline --graph
* 22a7a8c2 (HEAD -> feature_xyz) My commit 2
* efe2ccdc My commit 1
* 0d636cc9 (master) Hotfix for issue #something completely different
* efee9a2b Merge pull request #something
|\
| * ead7f309 A commit on master
|/
* 55001c8d Merge pull request #something else
Please note, that the hash of My commit 1
and
My commit 2
changed! That happened because their parent
changed. Therefore you have to force push your changes via
git push --force
next time you push your changes
upstream.
Most pull requests should be merged by creating a merge commit (the default on GitHub). Small pull requests (= only one can commit) can be rebased on top of master.
The master
branch is the current "main" development
branch. It is protected so that changes can be only included via
reviewed pull requests. New releases are made by tagging a specific
commit on master
.
Releases are tagged with a tag of the form
v$major.$minor
. The tag is not changed when changes are
backported.
For each release a branch of the form $major.$minor
should be created to store backported changes. It should be branched of
from master
at the commit which was tagged with
v$major.$minor
.
All other branches are development branches for pull requests, experiments, etc. They should be deleted once the pull request got merged or the branch is no longer useful.
Exiv2 team members can create branches for pull requests in the
main repository if they want to collaborate with others (e.g. for big
changes that require a lot of work). No one should not
push --force
in these branches without coordinating with
others and should only push --force-with-lease
.
When only one person will work on a pull request, then the branch can be created in their personal fork or in the main repository (note that branches in the main repository provide an automatic continuous integration).
We try to backport critical bugfixes to the latest released version on a best effort basis. We lack the man power to support older releases, but accept patches for these.
Bugfixes for crashes, memory corruptions, overflows and other potentially dangerous bugs must be backported. The same applies to bugfixes for issues that got a CVE assigned.
Since git is a fully distributed version control system, all changes
stay on your machine until you push them. Thus, if you are in doubt
whether a trickier step with git might screw up your repository, you can
simply create a backup of your whole exiv2 folder. In case the tricky
step went downhill, you can restore your working copy of exiv2 and no
one will ever know (unless you did a git push
)!
man git
and man git $command
interactive staging (for Emacs users: consider using magit for interactive staging)