Corentin Wallez | b395605 | 2021-04-21 18:05:01 +0000 | [diff] [blame] | 1 | # How to contribute to Dawn |
| 2 | |
| 3 | First off, we'd love to get your contributions to Dawn! |
| 4 | |
| 5 | Everything helps other folks using Dawn and WebGPU: from small fixes and documentation |
| 6 | improvements to larger features and optimizations. |
| 7 | Please read on to learn about the contribution process. |
| 8 | |
| 9 | ## One time setup |
| 10 | |
| 11 | ### Contributor License Agreement |
| 12 | |
| 13 | Contributions to this project must be accompanied by a Contributor License |
| 14 | Agreement. You (or your employer) retain the copyright to your contribution. |
| 15 | This simply gives us permission to use and redistribute your contributions as |
| 16 | part of the project. Head over to <https://cla.developers.google.com/> to see |
| 17 | your current agreements on file or to sign a new one. |
| 18 | |
| 19 | You generally only need to submit a CLA once, so if you've already submitted one |
| 20 | (even if it was for a different Google project), you probably don't need to do |
| 21 | it again. |
| 22 | |
| 23 | ### Gerrit setup |
| 24 | |
| 25 | Dawn's contributions are submitted and reviewed on [Dawn's Gerrit](https://dawn-review.googlesource.com). |
| 26 | |
| 27 | Gerrit works a bit differently than Github (if that's what you're used to): |
| 28 | there are no forks. Instead everyone works on the same repository. Gerrit has |
| 29 | magic branches for various purpose: |
| 30 | |
| 31 | - `refs/for/<branch>` (most commonly `refs/for/main`) is a branch that anyone |
| 32 | can push to that will create or update code reviews (called CLs for ChangeList) |
| 33 | for the commits pushed. |
| 34 | - `refs/changes/00/<change number>/<patchset>` is a branch that corresponds to |
| 35 | the commits that were pushed for codereview for "change number" at a certain |
| 36 | "patchset" (a new patchset is created each time you push to a CL). |
| 37 | |
| 38 | #### Gerrit's .gitcookies |
| 39 | |
| 40 | To push commits to Gerrit your `git` command needs to be authenticated. This is |
| 41 | done with `.gitcookies` that will make `git` send authentication information |
| 42 | when connecting to the remote. To get the `.gitcookies`, log-in to [Dawn's Gerrit](https://dawn-review.googlesource.com) |
| 43 | and browse to the [new-password](https://dawn.googlesource.com/new-password) |
| 44 | page that will give you shell/cmd commands to run to update `.gitcookie`. |
| 45 | |
| 46 | #### Set up the commit-msg hook |
| 47 | |
| 48 | Gerrit associates commits to CLs based on a `Change-Id:` tag in the commit |
| 49 | message. Each push with commits with a `Change-Id:` will update the |
| 50 | corresponding CL. |
| 51 | |
| 52 | To add the `commit-msg` hook that will automatically add a `Change-Id:` to your |
| 53 | commit messages, run the following command: |
| 54 | |
| 55 | ``` |
| 56 | f=`git rev-parse --git-dir`/hooks/commit-msg ; mkdir -p $(dirname $f) ; curl -Lo $f https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x $f |
| 57 | ``` |
| 58 | |
| 59 | Gerrit helpfully reminds you of that command if you forgot to set up the hook |
| 60 | before pushing commits. |
| 61 | |
| 62 | ## The code review process |
| 63 | |
| 64 | All submissions, including submissions by project members, require review. |
| 65 | |
| 66 | ### Discuss the change if needed |
| 67 | |
| 68 | Some changes are inherently risky, because they have long-term or architectural |
| 69 | consequences, contain a lot of unknowns or other reasons. When that's the case |
| 70 | it is better to discuss it on the [Dawn Matrix Channel](https://matrix.to/#/#webgpu-dawn:matrix.org) |
| 71 | or the [Dawn mailing-list](https://groups.google.com/g/dawn-graphics/members). |
| 72 | |
| 73 | ### Pushing changes to code review |
| 74 | |
| 75 | Before pushing changes to code review, it is better to run `git cl presubmit` |
| 76 | that will check the formatting of files and other small things. |
| 77 | |
| 78 | Pushing commits is done with `git push origin HEAD:refs/for/main`. Which means |
| 79 | push to `origin` (i.e. Gerrit) the currently checkout out commit to the |
| 80 | `refs/for/main` magic branch that creates or updates CLs. |
| 81 | |
Corentin Wallez | ac61042 | 2021-06-01 16:08:22 +0000 | [diff] [blame] | 82 | In the terminal you will see a URL where code review for this CL will happen. |
| 83 | CLs start in the "Work In Progress" state. To start the code review proper, |
| 84 | click on "Start Review", add reviewers and click "Send and start review". If |
| 85 | you are unsure which reviewers to use, pick one of the reviewers in the |
| 86 | [OWNERS file](../OWNERS) who will review or triage the CL. |
| 87 | |
Corentin Wallez | b395605 | 2021-04-21 18:05:01 +0000 | [diff] [blame] | 88 | When code review asks for changes in the commits, you can amend them any way |
| 89 | you want (small fixup commit and `git rebase -i` are crowd favorites) and run |
| 90 | the same `git push origin HEAD:refs/for/main` command. |
| 91 | |
| 92 | ### Tracking issues |
| 93 | |
| 94 | We usually like to have commits associated with issues in [Dawn's issue tracker](https://bugs.chromium.org/p/dawn/issues/list) |
| 95 | so that commits for the issue can all be found on the same page. This is done |
Corentin Wallez | ac61042 | 2021-06-01 16:08:22 +0000 | [diff] [blame] | 96 | by adding a `Bug: dawn:<issue number>` tag at the end of the commit message. It |
| 97 | is also possible to reference Chromium or Tint issues with |
| 98 | `Bug: tint:<issue number>` or `Bug: chromium:<issue number>`. |
Corentin Wallez | b395605 | 2021-04-21 18:05:01 +0000 | [diff] [blame] | 99 | |
| 100 | Some small fixes (like typo fixes, or some one-off maintenance) don't need a |
| 101 | tracking issue. When that's the case, it's good practice to call it out by |
| 102 | adding a `Bug: None` tag. |
| 103 | |
| 104 | It is possible to make issues fixed automatically when the CL is merged by |
| 105 | adding a `Fixed: <project>:<issue number>` tag in the commit message. |
| 106 | |
| 107 | ### Iterating on code review |
| 108 | |
| 109 | Dawn follows the general [Google code review guidelines](https://google.github.io/eng-practices/review/). |
| 110 | Most Dawn changes need reviews from two Dawn committers. Reviewers will set the |
| 111 | "Code Review" CR+1 or CR+2 label once the change looks good to them (although |
| 112 | it could still have comments that need to be addressed first). When addressing |
| 113 | comments, please mark them as "Done" if you just address them, or start a |
| 114 | discussion until they are resolved. |
| 115 | |
| 116 | Once you are granted rights (you can ask on your first contribution), you can |
| 117 | add the "Commit Queue" CQ+1 label to run the automated tests for Dawn. Once the |
| 118 | CL has CR+2 you can then add the CQ+2 label to run the automated tests and |
| 119 | submit the commit if they pass. |
| 120 | |
| 121 | The "Auto Submit" AS+1 label can be used to make Gerrit automatically set the |
| 122 | CQ+2 label once the CR+2 label is added. |