summaryrefslogtreecommitdiffstats
path: root/wiki/src/contribute/merge_policy/review.mdwn
blob: 3b74023e310d4acd79d43250ca09b8046e4758d7 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
[[!meta title="Git merge policy: review and merge process"]]

## Review

- When you start doing a review'n'merge, assign the relevant ticket to
  you, in order to avoid duplicated work.
- Merge the base branch (the one you're supposed to merge the reviewed
  topic branch into, as specified in `config/base_branch` and in the
  pull request -- they must match) into the topic branch.
- Build the topic branch and test the feature.
- Check automated ISO build and test results on
  <https://jenkins.tails.boum.org/>.
- Check the diff e.g. with `git log -p`. Beware of changes introduced
  by merge commits: either add the `--cc` option to `git log`, or use
  `git diff` after reviewing the individual patches.
- Check the APT suite with 
  `ssh reprepro@incoming.deb.tails.boum.org reprepro list [bugfix|feature]-<name-with-dashes>`
- Check the user and design documentation.
- Check the ticket.
- Changes proposed by new contributors, or by the patch'n'forget kind,
  shall respectively be applied once they are in *good enough* state.
  That is, once the core developers team feels like maintaining it on
  the long run in case the initial submitter disappears. Such
  maintenance includes: polishing the proposed changes to make them fit
  for release; writing and updating the design and end-user
  documentations; fixing bugs.
- Remember that it's hard to receive negative feedback. Don't forget
  to note the good parts, be constructive and precise in your
  comments, and never use reviews to make personal attacks. You can
  read these blog posts on review and feedback:
  - [Kate Heddleston: Criticism and ineffective feedback](https://www.kateheddleston.com/blog/criticism-and-ineffective-feedback) (blog post on how to do a good review)
  - [Liane Davey: Maybe you shouldn't give feedback](http://www.3coze.com/2016/07/17/bite-your-tongue/)

## Merge

Merge the branch with `--no-ff`:

- for a new feature: into `devel`
- for a fix on top on the last RC: into `testing`; then merge
  `testing` into `devel`
- for a fix on top of the last stable: into `stable`; then merge
  `stable` into `devel`

<a id="fix-committed"></a>

Please consider including `fix-committed: #NNNN` in the commit
message, _NNNN_ being the ticket number that is fixed by the branch
you are merging. Then, Redmine will automatically flag the
corresponding ticket as "Fix committed" once you push the results of
your merge to our main [[Git repository|contribute/Git]]. For example:

    Merge branch 'bugfix/8470-clean-up-apt-pinning' (Fix-committed: #8470)

## Book keeping

1. Update the *QA Check* field on the ticket. If there is no remaining
   tasks listed on the ticket, then change its status to *Fix
   committed* (unless you used the `fix-committed` keyword documented
   above); else, ask the branch submitter to split the remaining tasks
   into other tickets.
1. Push the updated branch to the master Git repository.
1. Reply to the email that requested the review, if any.