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