dan sinclair | d521d0e | 2022-04-10 22:14:00 +0000 | [diff] [blame] | 1 | # How to contribute |
Dan Sinclair | 6e58189 | 2020-03-02 15:47:43 -0500 | [diff] [blame] | 2 | |
dan sinclair | d521d0e | 2022-04-10 22:14:00 +0000 | [diff] [blame] | 3 | First off, we'd love to get your contributions. |
| 4 | |
| 5 | Everything helps other folks using Dawn, Tint and WebGPU: from small fixes and |
| 6 | documentation improvements to larger features and optimizations. Please read on |
| 7 | to learn about the contribution process. |
| 8 | |
| 9 | ## Contributor License Agreement |
| 10 | |
| 11 | Contributions to this project must be accompanied by a Contributor License |
| 12 | Agreement. You (or your employer) retain the copyright to your contribution; |
| 13 | this simply gives us permission to use and redistribute your contributions as |
| 14 | part of the project. Head over to <https://cla.developers.google.com/> to see |
| 15 | your current agreements on file or to sign a new one. |
| 16 | |
| 17 | You generally only need to submit a CLA once, so if you've already submitted one |
| 18 | (even if it was for a different project), you probably don't need to do it |
| 19 | again. |
| 20 | |
| 21 | ## Community Guidelines |
| 22 | |
| 23 | This project follows |
| 24 | [Google's Open Source Community Guidelines](https://opensource.google.com/conduct/). |
| 25 | |
| 26 | ## Code reviews |
| 27 | |
| 28 | All submissions, including submissions by project members, require review. We |
| 29 | use [Dawn's Gerrit](https://dawn-review.googlesource.com/) for this purpose. |
| 30 | |
| 31 | Any submissions to the [Tint](src/tint) folders should follow the |
| 32 | [Tint style guide](docs/tint/style_guide.md). |
| 33 | |
| 34 | |
| 35 | ### Discuss the change if needed |
| 36 | |
| 37 | Some changes are inherently risky, because they have long-term or architectural |
| 38 | consequences, contain a lot of unknowns or other reasons. When that's the case |
| 39 | it is better to discuss it on the [Dawn Matrix Channel](https://matrix.to/#/#webgpu-dawn:matrix.org) |
| 40 | or the [Dawn mailing-list](https://groups.google.com/g/dawn-graphics). |
| 41 | |
| 42 | ### Pushing changes to code review |
| 43 | |
| 44 | Before pushing changes to code review, it is better to run `git cl presubmit` |
| 45 | that will check the formatting of files and other small things. |
| 46 | |
| 47 | Pushing commits is done with `git push origin HEAD:refs/for/main`. Which means |
| 48 | push to `origin` (i.e. Gerrit) the currently checkout out commit to the |
| 49 | `refs/for/main` magic branch that creates or updates CLs. |
| 50 | |
| 51 | In the terminal you will see a URL where code review for this CL will happen. |
| 52 | CLs start in the "Work In Progress" state. To start the code review proper, |
| 53 | click on "Start Review", add reviewers and click "Send and start review". If |
| 54 | you are unsure which reviewers to use, pick one of the reviewers in the |
| 55 | [Dawn OWNERS file](src/dawn/OWNERS) or [Tint OWNERS file](src/tint/OWNERS) |
| 56 | who will review or triage the CL. |
| 57 | |
| 58 | When code review asks for changes in the commits, you can amend them any way |
| 59 | you want (small fixup commit and `git rebase -i` are crowd favorites) and run |
| 60 | the same `git push origin HEAD:refs/for/main` command. |
| 61 | |
| 62 | ### Tracking issues |
| 63 | |
| 64 | We usually like to have commits associated with issues in either |
| 65 | [Dawn's issue tracker](https://bugs.chromium.org/p/dawn/issues/list) or |
| 66 | [Tint's issue tracker](https://bugs.chromium.org/p/tint/issues/list) so that |
| 67 | commits for the issue can all be found on the same page. This is done |
| 68 | by adding a `Bug: dawn:<issue number>` or `Bug: tint:<issue number>` tag at the |
| 69 | end of the commit message. It is also possible to reference Chromium issues with |
| 70 | `Bug: chromium:<issue number>`. |
| 71 | |
| 72 | Some small fixes (like typo fixes, or some one-off maintenance) don't need a |
| 73 | tracking issue. When that's the case, it's good practice to call it out by |
| 74 | adding a `Bug: None` tag. |
| 75 | |
| 76 | It is possible to make issues fixed automatically when the CL is merged by |
| 77 | adding a `Fixed: <project>:<issue number>` tag in the commit message. |
| 78 | |
| 79 | ### Iterating on code review |
| 80 | |
| 81 | The project follows the general |
| 82 | [Google code review guidelines](https://google.github.io/eng-practices/review/). |
| 83 | Most changes need reviews from two committers. Reviewers will set the |
| 84 | "Code Review" CR+1 or CR+2 label once the change looks good to them (although |
| 85 | it could still have comments that need to be addressed first). When addressing |
| 86 | comments, please mark them as "Done" if you just address them, or start a |
| 87 | discussion until they are resolved. |
| 88 | |
| 89 | Once you are granted rights (you can ask on your first contribution), you can |
| 90 | add the "Commit Queue" CQ+1 label to run the automated tests. Once the |
| 91 | CL has CR+2 you can then add the CQ+2 label to run the automated tests and |
| 92 | submit the commit if they pass. |
| 93 | |
| 94 | The "Auto Submit" AS+1 label can be used to make Gerrit automatically set the |
| 95 | CQ+2 label once the CR+2 label is added. |
| 96 | |
| 97 | ## One-time Setup |
| 98 | |
| 99 | The project is setup to use Gerrit in a fashion similar to the Angle project. |
| 100 | If you're used to a more Chromium based control flow, see the |
| 101 | [Alternate setup](#alternate-setup) section below. |
| 102 | |
| 103 | ### Gerrit setup |
| 104 | |
| 105 | Gerrit works a bit differently than Github (if that's what you're used to): |
| 106 | there are no forks. Instead everyone works on the same repository. Gerrit has |
| 107 | magic branches for various purpose: |
| 108 | |
| 109 | - `refs/for/<branch>` (most commonly `refs/for/main`) is a branch that anyone |
| 110 | can push to that will create or update code reviews (called CLs for ChangeList) |
| 111 | for the commits pushed. |
| 112 | - `refs/changes/00/<change number>/<patchset>` is a branch that corresponds to |
| 113 | the commits that were pushed for codereview for "change number" at a certain |
| 114 | "patchset" (a new patchset is created each time you push to a CL). |
| 115 | |
| 116 | To create a Gerrit change for review, type: |
| 117 | |
| 118 | ```bash |
| 119 | git push origin HEAD:refs/for/main |
| 120 | ``` |
| 121 | |
| 122 | #### Gerrit's .gitcookies |
| 123 | |
| 124 | To push commits to Gerrit your `git` command needs to be authenticated. This is |
| 125 | done with `.gitcookies` that will make `git` send authentication information |
| 126 | when connecting to the remote. To get the `.gitcookies`, log-in to |
| 127 | [Dawn's Gerrit](https://dawn-review.googlesource.com) and browse to the |
| 128 | [new-password](https://dawn.googlesource.com/new-password) page that will give |
| 129 | you shell/cmd commands to run to update `.gitcookie`. |
| 130 | |
| 131 | #### Set up the commit-msg hook |
| 132 | |
| 133 | Gerrit associates commits to CLs based on a `Change-Id:` tag in the commit |
| 134 | message. Each push with commits with a `Change-Id:` will update the |
| 135 | corresponding CL. |
| 136 | |
| 137 | To add the `commit-msg` hook that will automatically add a `Change-Id:` to your |
| 138 | commit messages, run the following command: |
| 139 | |
| 140 | ``` |
| 141 | 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 |
| 142 | ``` |
| 143 | |
| 144 | Gerrit helpfully reminds you of that command if you forgot to set up the hook |
| 145 | before pushing commits. |
| 146 | |
| 147 | ### Alternate setup |
| 148 | In order to get a more Chromium style workflow there are couple changes need. |
| 149 | |
| 150 | 1. Verify there is `.git/hooks/commit-msg` hook setup. (Just moving it to a |
| 151 | `commit-msg.bak` will suffice) |
| 152 | 2. Add `override-squash-uploads = True` to the `gerrit` section of your |
| 153 | `.git/config` file |
| 154 | |
| 155 | With those changes, a `Commit-Id` should not be auto-matically appended to your |
| 156 | CLs and `git cl upload` needs to be used to push changes to Gerrit. During |
| 157 | code review you can commit to your branch as usual, no need to amend. |
| 158 | |
| 159 | This will also allow `git cl status` to work as expected without having to |
| 160 | specifically set the issue number for the branch. |