CONTRIBUTING.md

   1 Contributing to Wayland
   2 =======================
   3
   4 Sending patches
   5 ---------------
   6
   7 Patches should be sent via
   8 [GitLab merge requests](https://docs.gitlab.com/ce/gitlab-basics/add-merge-request.html).
   9 Wayland is
  10 [hosted on freedesktop.org's GitLab](https://gitlab.freedesktop.org/wayland/wayland/):
  11 in order to submit code, you should create an account on this GitLab instance,
  12 fork the core Wayland repository, push your changes to a branch in your new
  13 repository, and then submit these patches for review through a merge request.
  14
  15 Wayland formerly accepted patches via `git-send-email`, sent to
  16 **wayland-devel@lists.freedesktop.org**; these were
  17 [tracked using Patchwork](https://patchwork.freedesktop.org/project/wayland/).
  18 Some old patches continue to be sent this way, and we may accept small new
  19 patches sent to the list, but please send all new patches through GitLab merge
  20 requests.
  21
  22
  23 Formatting and separating commits
  24 ---------------------------------
  25
  26 Unlike many projects using GitHub and GitLab, Wayland has a
  27 [linear, 'recipe' style history](http://www.bitsnbites.eu/git-history-work-log-vs-recipe/).
  28 This means that every commit should be small, digestible, stand-alone, and
  29 functional. Rather than a purely chronological commit history like this:
  30
  31     connection: plug a fd leak
  32     plug another fd leak
  33     connection: init fds to -1
  34     close all fds
  35     refactor checks into a new function
  36     don't close fds we handed out
  37
  38 we aim to have a clean history which only reflects the final state, broken up
  39 into functional groupings:
  40
  41     connection: Refactor out closure allocation
  42     connection: Clear fds we shouldn't close to -1
  43     connection: Make wl_closure_destroy() close fds of undispatched closures
  44
  45 This ensures that the final patch series only contains the final state,
  46 without the changes and missteps taken along the development process.
  47
  48 The first line of a commit message should contain a prefix indicating
  49 what part is affected by the patch followed by one sentence that
  50 describes the change. For examples:
  51
  52     protocol: Support scaled outputs and surfaces
  53
  54 and
  55
  56     doc: generate server documentation from XML too
  57
  58 If in doubt what prefix to use, look at other commits that change the
  59 same file(s) as the patch being sent.
  60
  61 The body of the commit message should describe what the patch changes
  62 and why, and also note any particular side effects. This shouldn't be
  63 empty on most of the cases. It shouldn't take a lot of effort to write
  64 a commit message for an obvious change, so an empty commit message
  65 body is only acceptable if the questions "What?" and "Why?" are already
  66 answered on the one-line summary.
  67
  68 The lines of the commit message should have at most 76 characters, to
  69 cope with the way git log presents them.
  70
  71 See [notes on commit messages] for a recommended reading on writing commit
  72 messages.
  73
  74 Your patches should also include a Signed-off-by line with your name and
  75 email address.  If you're not the patch's original author, you should
  76 also gather S-o-b's by them (and/or whomever gave the patch to you.) The
  77 significance of this is that it certifies that you created the patch,
  78 that it was created under an appropriate open source license, or
  79 provided to you under those terms.  This lets us indicate a chain of
  80 responsibility for the copyright status of the code.
  81
  82 We won't reject patches that lack S-o-b, but it is strongly recommended.
  83
  84 When you re-send patches, revised or not, it would be very good to document the
  85 changes compared to the previous revision in the commit message and/or the
  86 merge request. If you have already received Reviewed-by or Acked-by tags, you
  87 should evaluate whether they still apply and include them in the respective
  88 commit messages. Otherwise the tags may be lost, reviewers miss the credit they
  89 deserve, and the patches may cause redundant review effort.
  90
  91
  92 Tracking patches and following up
  93 ---------------------------------
  94
  95 Once submitted to GitLab, your patches will be reviewed by the Wayland
  96 development team on GitLab. Review may be entirely positive and result in your
  97 code landing instantly, in which case, great! You're done. However, we may ask
  98 you to make some revisions: fixing some bugs we've noticed, working to a
  99 slightly different design, or adding documentation and tests.
 100
 101 If you do get asked to revise the patches, please bear in mind the notes above.
 102 You should use `git rebase -i` to make revisions, so that your patches follow
 103 the clear linear split documented above. Following that split makes it easier
 104 for reviewers to understand your work, and to verify that the code you're
 105 submitting is correct.
 106
 107 A common request is to split single large patch into multiple patches. This can
 108 happen, for example, if when adding a new feature you notice a bug elsewhere
 109 which you need to fix to progress. Separating these changes into separate
 110 commits will allow us to verify and land the bugfix quickly, pushing part of
 111 your work for the good of everyone, whilst revision and discussion continues on
 112 the larger feature part. It also allows us to direct you towards reviewers who
 113 best understand the different areas you are working on.
 114
 115 When you have made any requested changes, please rebase the commits, verify
 116 that they still individually look good, then force-push your new branch to
 117 GitLab. This will update the merge request and notify everyone subscribed to
 118 your merge request, so they can review it again.
 119
 120 There are also
 121 [many GitLab CLI clients](https://about.gitlab.com/applications/#cli-clients),
 122 if you prefer to avoid the web interface. It may be difficult to follow review
 123 comments without using the web interface though, so we do recommend using this
 124 to go through the review process, even if you use other clients to track the
 125 list of available patches.
 126
 127
 128 Coding style
 129 ------------
 130
 131 You should follow the style of the file you're editing. In general, we
 132 try to follow the rules below.
 133
 134 **Note: this file uses spaces due to markdown rendering issues for tabs.
 135   Code must be implemented using tabs.**
 136
 137 - indent with tabs, and a tab is always 8 characters wide
 138 - opening braces are on the same line as the if statement;
 139 - no braces in an if-body with just one statement;
 140 - if one of the branches of an if-else condition has braces, then the
 141   other branch should also have braces;
 142 - there is always an empty line between variable declarations and the
 143   code;
 144
 145 ```c
 146 static int
 147 my_function(void)
 148 {
 149         int a = 0;
 150
 151         if (a)
 152                 b();
 153         else
 154                 c();
 155
 156         if (a) {
 157                 b();
 158                 c();
 159         } else {
 160                 d();
 161         }
 162 }
 163 ```
 164
 165 - lines should be less than 80 characters wide;
 166 - when breaking lines with functions calls, the parameters are aligned
 167   with the opening parentheses;
 168 - when assigning a variable with the result of a function call, if the
 169   line would be longer we break it around the equal '=' sign if it makes
 170   sense;
 171
 172 ```c
 173         long_variable_name =
 174                 function_with_a_really_long_name(parameter1, parameter2,
 175                                                  parameter3, parameter4);
 176
 177         x = function_with_a_really_long_name(parameter1, parameter2,
 178                                              parameter3, parameter4);
 179 ```
 180
 181 Conduct
 182 =======
 183
 184 As a freedesktop.org project, Wayland follows the Contributor Covenant,
 185 found at:
 186 https://www.freedesktop.org/wiki/CodeOfConduct
 187
 188 Please conduct yourself in a respectful and civilised manner when
 189 interacting with community members on mailing lists, IRC, or bug
 190 trackers. The community represents the project as a whole, and abusive
 191 or bullying behaviour is not tolerated by the project.
 192
 193
 194 Licensing
 195 =========
 196
 197 Wayland is licensed with the intention to be usable anywhere X.org is.
 198 Originally, X.org was covered under the MIT X11 license, but changed to
 199 the MIT Expat license.  Similarly, Wayland was covered initially as MIT
 200 X11 licensed, but changed to the MIT Expat license, following in X.org's
 201 footsteps.  Other than wording, the two licenses are substantially the
 202 same, with the exception of a no-advertising clause in X11 not included
 203 in Expat.
 204
 205 New source code files should specify the MIT Expat license in their
 206 boilerplate, as part of the copyright statement.
 207
 208
 209 Review
 210 ======
 211
 212 All patches, even trivial ones, require at least one positive review
 213 (Reviewed-by). Additionally, if no Reviewed-by's have been given by
 214 people with commit access, there needs to be at least one Acked-by from
 215 someone with commit access. A person with commit access is expected to be
 216 able to evaluate the patch with respect to the project scope and architecture.
 217
 218 The below review guidelines are intended to be interpreted in spirit, not by
 219 the letter. There may be circumstances where some guidelines are better
 220 ignored. We rely very much on the judgement of reviewers and commit rights
 221 holders.
 222
 223 During review, the following matters should be checked:
 224
 225 - The commit message explains why the change is being made.
 226
 227 - The code fits the project's scope.
 228
 229 - The code license is the same MIT licence the project generally uses.
 230
 231 - Stable ABI or API is not broken.
 232
 233 - Stable ABI or API additions must be justified by actual use cases, not only
 234 by speculation. They must also be documented, and it is strongly recommended to
 235 include tests exercising the additions in the test suite.
 236
 237 - The code fits the existing software architecture, e.g. no layering
 238 violations.
 239
 240 - The code is correct and does not introduce new failures for existing users,
 241 does not add new corner-case bugs, and does not introduce new compiler
 242 warnings.
 243
 244 - The patch does what it says in the commit message and changes nothing else.
 245
 246 - The patch is a single logical change. If the commit message addresses
 247 multiple points, it is a hint that the commit might need splitting up.
 248
 249 - A bug fix should target the underlying root cause instead of hiding symptoms.
 250 If a complete fix is not practical, partial fixes are acceptable if they come
 251 with code comments and filed Gitlab issues for the remaining bugs.
 252
 253 - The bug root cause rule applies to external software components as well, e.g.
 254 do not work around kernel driver issues in userspace.
 255
 256 - The test suite passes.
 257
 258 - The code does not depend on API or ABI which has no working free open source
 259 implementation.
 260
 261 - The code is not dead or untestable. E.g. if there are no free open source
 262 software users for it then it is effectively dead code.
 263
 264 - The code is written to be easy to understand, or if code cannot be clear
 265 enough on its own there are code comments to explain it.
 266
 267 - The code is minimal, i.e. prefer refactor and re-use when possible unless
 268 clarity suffers.
 269
 270 - The code adheres to the style guidelines.
 271
 272 - In a patch series, every intermediate step adheres to the above guidelines.
 273
 274
 275 Commit rights
 276 =============
 277
 278 Commit rights will be granted to anyone who requests them and fulfills the
 279 below criteria:
 280
 281 - Submitted some (10 as a rule of thumb) non-trivial (not just simple
 282   spelling fixes and whitespace adjustment) patches that have been merged
 283   already.
 284
 285 - Are actively participating in public discussions about their work (on the
 286   mailing list or IRC). This should not be interpreted as a requirement to
 287   review other peoples patches but just make sure that patch submission isn't
 288   one-way communication. Cross-review is still highly encouraged.
 289
 290 - Will be regularly contributing further patches. This includes regular
 291   contributors to other parts of the open source graphics stack who only
 292   do the occasional development in this project.
 293
 294 - Agrees to use their commit rights in accordance with the documented merge
 295   criteria, tools, and processes.
 296
 297 To apply for commit rights, create a new issue in gitlab for the respective
 298 project and give it the "accounts" label.
 299
 300 Committers are encouraged to request their commit rights get removed when they
 301 no longer contribute to the project. Commit rights will be reinstated when they
 302 come back to the project.
 303
 304 Maintainers and committers should encourage contributors to request commit
 305 rights, especially junior contributors tend to underestimate their skills.
 306
 307
 308 Stabilising for releases
 309 ========================
 310
 311 A release cycle ends with a stable release which also starts a new cycle and
 312 lifts any code freezes. Gradual code freezing towards a stable release starts
 313 with an alpha release. The release stages of a cycle are:
 314
 315 - **Alpha release**:
 316     Signified by version number #.#.91.
 317     Major features must have landed before this. Major features include
 318     invasive code motion and refactoring, high risk changes, and new stable
 319     library ABI.
 320
 321 - **Beta release**:
 322     Signified by version number #.#.92.
 323     Minor features must have landed before this. Minor features include all
 324     new features that are not major, low risk changes, clean-ups, and
 325     documentation. Stable ABI that was new in the alpha release can be removed
 326     before a beta release if necessary.
 327
 328 - **Release candidates (RC)**:
 329     Signified by version number #.#.93 and up to #.#.99.
 330     Bug fixes that are not release critical must have landed before this.
 331     Release critical bug fixes can still be landed after this, but they may
 332     call for another RC.
 333
 334 - **Stable release**:
 335     Signified by version number #.#.0.
 336     Ideally no changes since the last RC.
 337
 338 Mind that version #.#.90 is never released. It is used during development when
 339 no code freeze is in effect. Stable branches and point releases are not covered
 340 by the above.
 341
 342
 343 [git documentation]: http://git-scm.com/documentation
 344 [notes on commit messages]: http://who-t.blogspot.de/2009/12/on-commit-messages.html