Blame GIT_GUIDELINES.md

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