CONTRIBUTING.md 14.9 KB
Newer Older
1
Contributing to Weston
Pekka Paalanen's avatar
Pekka Paalanen committed
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
Finding something to work on
----------------------------

Weston's development is [tracked on GitLab](https://gitlab.freedesktop.org/wayland/weston).
In addition to reviewing code submissions (see below), we use the issue tracker
to discuss both bugfixes and development of new features.

The '[good for new contributors](https://gitlab.freedesktop.org/wayland/weston/issues?label_name%5B%5D=Good+for+new+contributors)'
label is used for issues the development team thinks are a good place to begin
working on Weston. These issues cover features or bugfixes which are small,
self-contained, don't require much specific background knowledge, and aren't
blocked by more complex work.

If you have picked an issue you would like to work on, you may want to mention
in the issue tracker that you would like to pick it up. You can also discuss
it with the developers in the issue tracker, or on the
[mailing list](https://lists.freedesktop.org/mailman/listinfo/wayland-devel).
Many developers also use IRC through [Freenode](https://freenode.net)'s
`#wayland` channel; however you may need to wait some time for a response on
IRC, which requires keeping your client connected. If you cannot stay for a
long time (potentially some hours due to timezone differences), then you
may want to send your question to the list or issue tracker instead.


Pekka Paalanen's avatar
Pekka Paalanen committed
28
29
30
Sending patches
---------------

31
32
33
34
35
36
37
38
39
Patches should be sent via
[GitLab merge requests](https://docs.gitlab.com/ce/gitlab-basics/add-merge-request.html).
Weston is
[hosted on freedesktop.org's GitLab](https://gitlab.freedesktop.org/wayland/weston/):
in order to submit code, you should create an account on this GitLab instance,
fork the core Weston repository, push your changes to a branch in your new
repository, and then submit these patches for review through a merge request.

Weston formerly accepted patches via `git-send-email`, sent to
40
**wayland-devel\@lists.freedesktop.org**; these were
41
[tracked using Patchwork](https://patchwork.freedesktop.org/projects/wayland/).
42
43
New email patches are no longer accepted.

44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71

Formatting and separating commits
---------------------------------

Unlike many projects using GitHub and GitLab, Weston has a
[linear, 'recipe' style history](http://www.bitsnbites.eu/git-history-work-log-vs-recipe/).
This means that every commit should be small, digestible, stand-alone, and
functional. Rather than a purely chronological commit history like this:

    doc: final docs for view transforms
    fix tests when disabled, redo broken doc formatting
    better transformed-view iteration (thanks Hannah!)
    try to catch more cases in tests
    tests: add new spline test
    fix compilation on splines
    doc: notes on reticulating splines
    compositor: add spline reticulation for view transforms

we aim to have a clean history which only reflects the final state, broken up
into functional groupings:

    compositor: add spline reticulation for view transforms
    compositor: new iterator for view transforms
    tests: add view-transform correctness tests
    doc: fix Doxygen formatting for view transforms

This ensures that the final patch series only contains the final state,
without the changes and missteps taken along the development process.
Pekka Paalanen's avatar
Pekka Paalanen committed
72
73
74
75
76

The first line of a commit message should contain a prefix indicating
what part is affected by the patch followed by one sentence that
describes the change. For examples:

77
    compositor-drm: Support modifiers for drm_fb
Pekka Paalanen's avatar
Pekka Paalanen committed
78
79
80

and

81
    input: do not forward unmatched touch-ups
Pekka Paalanen's avatar
Pekka Paalanen committed
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98

If in doubt what prefix to use, look at other commits that change the
same file(s) as the patch being sent.

The body of the commit message should describe what the patch changes
and why, and also note any particular side effects. This shouldn't be
empty on most of the cases. It shouldn't take a lot of effort to write
a commit message for an obvious change, so an empty commit message
body is only acceptable if the questions "What?" and "Why?" are already
answered on the one-line summary.

The lines of the commit message should have at most 76 characters, to
cope with the way git log presents them.

See [notes on commit messages] for a recommended reading on writing commit
messages.

99
100
Your patches must also include a Signed-off-by line with your name
(or pseudonym) and email address which indicates that you agree to the
Pekka Paalanen's avatar
Pekka Paalanen committed
101
102
[Developer's Certificate of Origin 1.1](DCO-1.1.txt).
If you're not the patch's original author, you should
103
104
also gather S-o-b's from them (and/or whomever gave the patch to you) in
addition to your own S-o-b. The
Pekka Paalanen's avatar
Pekka Paalanen committed
105
106
107
108
109
significance of this is that it certifies that you created the patch,
that it was created under an appropriate open source license, or
provided to you under those terms.  This lets us indicate a chain of
responsibility for the copyright status of the code.

110
111
112
**Agreeing to DCO 1.1 is mandatory.** Patches without a Signed-off-by cannot
be accepted, but using a pseudonym is fine as long as the email address is
yours personally.
Pekka Paalanen's avatar
Pekka Paalanen committed
113
114
115

When you re-send patches, revised or not, it would be very good to document the
changes compared to the previous revision in the commit message and/or the
116
merge request. If you have already received Reviewed-by or Acked-by tags, you
Pekka Paalanen's avatar
Pekka Paalanen committed
117
118
119
120
121
122
123
124
should evaluate whether they still apply and include them in the respective
commit messages. Otherwise the tags may be lost, reviewers miss the credit they
deserve, and the patches may cause redundant review effort.


Tracking patches and following up
---------------------------------

125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
Once submitted to GitLab, your patches will be reviewed by the Weston
development team on GitLab. Review may be entirely positive and result in your
code landing instantly, in which case, great! You're done. However, we may ask
you to make some revisions: fixing some bugs we've noticed, working to a
slightly different design, or adding documentation and tests.

If you do get asked to revise the patches, please bear in mind the notes above.
You should use `git rebase -i` to make revisions, so that your patches follow
the clear linear split documented above. Following that split makes it easier
for reviewers to understand your work, and to verify that the code you're
submitting is correct.

A common request is to split single large patch into multiple patches. This can
happen, for example, if when adding a new feature you notice a bug in Weston's
core which you need to fix to progress. Separating these changes into separate
commits will allow us to verify and land the bugfix quickly, pushing part of
your work for the good of everyone, whilst revision and discussion continues on
the larger feature part. It also allows us to direct you towards reviewers who
best understand the different areas you are working on.

When you have made any requested changes, please rebase the commits, verify
that they still individually look good, then force-push your new branch to
GitLab. This will update the merge request and notify everyone subscribed to
your merge request, so they can review it again.

There are also
[many GitLab CLI clients](https://about.gitlab.com/applications/#cli-clients),
if you prefer to avoid the web interface. It may be difficult to follow review
comments without using the web interface though, so we do recommend using this
to go through the review process, even if you use other clients to track the
list of available patches.
Pekka Paalanen's avatar
Pekka Paalanen committed
156
157
158
159
160
161
162
163
164


Coding style
------------

You should follow the style of the file you're editing. In general, we
try to follow the rules below.

**Note: this file uses spaces due to markdown rendering issues for tabs.
165
  Code must be indented using tabs.**
Pekka Paalanen's avatar
Pekka Paalanen committed
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226

- indent with tabs, and a tab is always 8 characters wide
- opening braces are on the same line as the if statement;
- no braces in an if-body with just one statement;
- if one of the branches of an if-else condition has braces, then the
  other branch should also have braces;
- there is always an empty line between variable declarations and the
  code;

```c
static int
my_function(void)
{
        int a = 0;

        if (a)
                b();
        else
                c();

        if (a) {
                b();
                c();
        } else {
                d();
        }
}
```

- lines should be less than 80 characters wide;
- when breaking lines with functions calls, the parameters are aligned
  with the opening parentheses;
- when assigning a variable with the result of a function call, if the
  line would be longer we break it around the equal '=' sign if it makes
  sense;

```c
        long_variable_name =
                function_with_a_really_long_name(parameter1, parameter2,
                                                 parameter3, parameter4);

        x = function_with_a_really_long_name(parameter1, parameter2,
                                             parameter3, parameter4);
```

Conduct
=======

As a freedesktop.org project, Wayland follows the Contributor Covenant,
found at:
https://www.freedesktop.org/wiki/CodeOfConduct

Please conduct yourself in a respectful and civilised manner when
interacting with community members on mailing lists, IRC, or bug
trackers. The community represents the project as a whole, and abusive
or bullying behaviour is not tolerated by the project.


Licensing
=========

227
Weston is licensed with the intention to be usable anywhere X.org is.
Pekka Paalanen's avatar
Pekka Paalanen committed
228
Originally, X.org was covered under the MIT X11 license, but changed to
229
the MIT Expat license.  Similarly, Weston was covered initially as MIT
Pekka Paalanen's avatar
Pekka Paalanen committed
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
X11 licensed, but changed to the MIT Expat license, following in X.org's
footsteps.  Other than wording, the two licenses are substantially the
same, with the exception of a no-advertising clause in X11 not included
in Expat.

New source code files should specify the MIT Expat license in their
boilerplate, as part of the copyright statement.


Review
======

All patches, even trivial ones, require at least one positive review
(Reviewed-by). Additionally, if no Reviewed-by's have been given by
people with commit access, there needs to be at least one Acked-by from
someone with commit access. A person with commit access is expected to be
able to evaluate the patch with respect to the project scope and architecture.

The below review guidelines are intended to be interpreted in spirit, not by
the letter. There may be circumstances where some guidelines are better
ignored. We rely very much on the judgement of reviewers and commit rights
holders.

During review, the following matters should be checked:

- The commit message explains why the change is being made.

- The code fits the project's scope.

- The code license is the same MIT licence the project generally uses.

- Stable ABI or API is not broken.

- Stable ABI or API additions must be justified by actual use cases, not only
by speculation. They must also be documented, and it is strongly recommended to
265
include tests exercising the additions in the test suite.
Pekka Paalanen's avatar
Pekka Paalanen committed
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374

- The code fits the existing software architecture, e.g. no layering
violations.

- The code is correct and does not introduce new failures for existing users,
does not add new corner-case bugs, and does not introduce new compiler
warnings.

- The patch does what it says in the commit message and changes nothing else.

- The patch is a single logical change. If the commit message addresses
multiple points, it is a hint that the commit might need splitting up.

- A bug fix should target the underlying root cause instead of hiding symptoms.
If a complete fix is not practical, partial fixes are acceptable if they come
with code comments and filed Gitlab issues for the remaining bugs.

- The bug root cause rule applies to external software components as well, e.g.
do not work around kernel driver issues in userspace.

- The test suite passes.

- The code does not depend on API or ABI which has no working free open source
implementation.

- The code is not dead or untestable. E.g. if there are no free open source
software users for it then it is effectively dead code.

- The code is written to be easy to understand, or if code cannot be clear
enough on its own there are code comments to explain it.

- The code is minimal, i.e. prefer refactor and re-use when possible unless
clarity suffers.

- The code adheres to the style guidelines.

- In a patch series, every intermediate step adheres to the above guidelines.


Commit rights
=============

Commit rights will be granted to anyone who requests them and fulfills the
below criteria:

- Submitted some (10 as a rule of thumb) non-trivial (not just simple
  spelling fixes and whitespace adjustment) patches that have been merged
  already.

- Are actively participating in public discussions about their work (on the
  mailing list or IRC). This should not be interpreted as a requirement to
  review other peoples patches but just make sure that patch submission isn't
  one-way communication. Cross-review is still highly encouraged.

- Will be regularly contributing further patches. This includes regular
  contributors to other parts of the open source graphics stack who only
  do the occasional development in this project.

- Agrees to use their commit rights in accordance with the documented merge
  criteria, tools, and processes.

To apply for commit rights, create a new issue in gitlab for the respective
project and give it the "accounts" label.

Committers are encouraged to request their commit rights get removed when they
no longer contribute to the project. Commit rights will be reinstated when they
come back to the project.

Maintainers and committers should encourage contributors to request commit
rights, especially junior contributors tend to underestimate their skills.


Stabilising for releases
========================

A release cycle ends with a stable release which also starts a new cycle and
lifts any code freezes. Gradual code freezing towards a stable release starts
with an alpha release. The release stages of a cycle are:

- **Alpha release**:
    Signified by version number #.#.91.
    Major features must have landed before this. Major features include
    invasive code motion and refactoring, high risk changes, and new stable
    library ABI.

- **Beta release**:
    Signified by version number #.#.92.
    Minor features must have landed before this. Minor features include all
    new features that are not major, low risk changes, clean-ups, and
    documentation. Stable ABI that was new in the alpha release can be removed
    before a beta release if necessary.

- **Release candidates (RC)**:
    Signified by version number #.#.93 and up to #.#.99.
    Bug fixes that are not release critical must have landed before this.
    Release critical bug fixes can still be landed after this, but they may
    call for another RC.

- **Stable release**:
    Signified by version number #.#.0.
    Ideally no changes since the last RC.

Mind that version #.#.90 is never released. It is used during development when
no code freeze is in effect. Stable branches and point releases are not covered
by the above.


[git documentation]: http://git-scm.com/documentation
[notes on commit messages]: http://who-t.blogspot.de/2009/12/on-commit-messages.html