1# Contributing
2
3We welcome contributions from the community. Please read the following guidelines carefully to
4maximize the chances of your PR being merged.
5
6## Communication
7
8* Before starting work on a major feature, please reach out to us via GitHub, Slack,
9 email, etc. We will make sure no one else is already working on it and ask you to open a
10 GitHub issue.
11* A "major feature" is defined as any change that is > 100 LOC altered (not including tests), or
12 changes any user-facing behavior. We will use the GitHub issue to discuss the feature and come to
13 agreement. This is to prevent your time being wasted, as well as ours. The GitHub review process
14 for major features is also important so that [organizations with commit access](OWNERS.md) can
15 come to agreement on design. If it is appropriate to write a design document, the document must
16 be hosted either in the GitHub tracking issue, or linked to from the issue and hosted in a
17 world-readable location.
18* Small patches and bug fixes don't need prior communication.
19
20## Inclusive language policy
21
22The Envoy community has an explicit goal to be inclusive to all. As such, all PRs must adhere to the
23following guidelines for all code, APIs, and documentation:
24
25* The following words and phrases are not allowed:
26 * *Whitelist*: use allowlist instead.
27 * *Blacklist*: use denylist or blocklist instead.
28 * *Master*: use primary instead.
29 * *Slave*: use secondary or replica instead.
30* Documentation should be written in an inclusive style. The [Google developer
31 documentation](https://developers.google.com/style/inclusive-documentation) contains an excellent
32 reference on this topic.
33* The above policy is not considered definitive and may be amended in the future as industry best
34 practices evolve. Additional comments on this topic may be provided by maintainers during code
35 review.
36
37## Submitting a PR
38
39* Fork the repo.
40* Create your PR.
41* Tests will automatically run for you.
42* We will **not** merge any PR that is not passing tests.
43* PRs are expected to have 100% test coverage for added code. This can be verified with a coverage
44 build. If your PR cannot have 100% coverage for some reason please clearly explain why when you
45 open it.
46* Any PR that changes user-facing behavior **must** have associated documentation in [docs](docs) as
47 well as the [changelog](CHANGELOG.md).
48* All code comments and documentation are expected to have proper English grammar and punctuation.
49 If you are not a fluent English speaker (or a bad writer ;-)) please let us know and we will try
50 to find some help but there are no guarantees.
51* Your PR title should be descriptive, and generally start with a subsystem name followed by a
52 colon. Examples:
53 * "docs: fix grammar error"
54 * "http conn man: add new feature"
55* Your PR commit message will be used as the commit message when your PR is merged. You should
56 update this field if your PR diverges during review.
57* Your PR description should have details on what the PR does. If it fixes an existing issue it
58 should end with "Fixes #XXX".
59* If your PR is co-authored or based on an earlier PR from another contributor,
60 please attribute them with `Co-authored-by: name <name@example.com>`. See
61 GitHub's [multiple author
62 guidance](https://help.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors)
63 for further details.
64* When all of the tests are passing and all other conditions described herein are satisfied, a
65 maintainer will be assigned to review and merge the PR.
66* Once you submit a PR, *please do not rebase it*. It's much easier to review if subsequent commits
67 are new commits and/or merges. We squash rebase the final merged commit so the number of commits
68 you have in the PR don't matter.
69* We expect that once a PR is opened, it will be actively worked on until it is merged or closed.
70 We reserve the right to close PRs that are not making progress. This is generally defined as no
71 changes for 7 days. Obviously PRs that are closed due to lack of activity can be reopened later.
72 Closing stale PRs helps us to keep on top of all of the work currently in flight.
73
74## PR review policy for maintainers
75
76* See [OWNERS.md](OWNERS.md) for the current list of maintainers.
77* It is generally expected that a senior maintainer should review every PR.
78* It is also generally expected that a "domain expert" for the code the PR touches should review the
79 PR. This person does not necessarily need to have commit access.
80* The previous two points generally mean that every PR should have two approvals. (Exceptions can
81 be made by the senior maintainers).
82* The above rules may be waived for PRs which only update docs or comments, or trivial changes to
83 tests and tools (where trivial is decided by the maintainer in question).
84* In general, we should also attempt to make sure that at least one of the approvals is *from an
85 organization different from the PR author.* E.g., if Lyft authors a PR, at least one approver
86 should be from an organization other than Lyft. This helps us make sure that we aren't putting
87 organization specific shortcuts into the code.
88* If there is a question on who should review a PR please discuss in Slack.
89* Anyone is welcome to review any PR that they want, whether they are a maintainer or not.
90* Please make sure that the PR title, commit message, and description are updated if the PR changes
91 significantly during review.
92* Please **clean up the title and body** before merging. By default, GitHub fills the squash merge
93 title with the original title, and the commit body with every individual commit from the PR.
94 The maintainer doing the merge should make sure the title follows the guidelines above and should
95 overwrite the body with the original commit message from the PR (cleaning it up if necessary)
96 while preserving the PR author's final DCO sign-off.
97
98## DCO: Sign your work
99
100Envoy ships commit hooks that allow you to auto-generate the DCO signoff line if
101it doesn't exist when you run `git commit`. Simply navigate to the Envoy project
102root and run:
103
104```bash
105./support/bootstrap
106```
107
108From here, simply commit as normal, and you will see the signoff at the bottom
109of each commit.
110
111The sign-off is a simple line at the end of the explanation for the
112patch, which certifies that you wrote it or otherwise have the right to
113pass it on as an open-source patch. The rules are pretty simple: if you
114can certify the below (from
115[developercertificate.org](https://developercertificate.org/)):
116
117```
118Developer Certificate of Origin
119Version 1.1
120
121Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
122660 York Street, Suite 102,
123San Francisco, CA 94110 USA
124
125Everyone is permitted to copy and distribute verbatim copies of this
126license document, but changing it is not allowed.
127
128
129Developer's Certificate of Origin 1.1
130
131By making a contribution to this project, I certify that:
132
133(a) The contribution was created in whole or in part by me and I
134 have the right to submit it under the open source license
135 indicated in the file; or
136
137(b) The contribution is based upon previous work that, to the best
138 of my knowledge, is covered under an appropriate open source
139 license and I have the right under that license to submit that
140 work with modifications, whether created in whole or in part
141 by me, under the same open source license (unless I am
142 permitted to submit under a different license), as indicated
143 in the file; or
144
145(c) The contribution was provided directly to me by some other
146 person who certified (a), (b) or (c) and I have not modified
147 it.
148
149(d) I understand and agree that this project and the contribution
150 are public and that a record of the contribution (including all
151 personal information I submit with it, including my sign-off) is
152 maintained indefinitely and may be redistributed consistent with
153 this project or the open source license(s) involved.
154```
155
156then you just add a line to every git commit message:
157
158 Signed-off-by: Joe Smith <joe@gmail.com>
159
160using your real name (sorry, no pseudonyms or anonymous contributions.)
161
162You can add the sign off when creating the git commit via `git commit -s`.
163
164If you want this to be automatic you can set up some aliases:
165
166```bash
167git config --add alias.amend "commit -s --amend"
168git config --add alias.c "commit -s"
169```
170
171## Fixing DCO
172
173If your PR fails the DCO check, it's necessary to fix the entire commit history in the PR. Best
174practice is to [squash](https://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html)
175the commit history to a single commit, append the DCO sign-off as described above, and [force
176push](https://git-scm.com/docs/git-push#git-push---force). For example, if you have 2 commits in
177your history:
178
179```bash
180git rebase -i HEAD^^
181(interactive squash + DCO append)
182git push origin -f
183```
184
185Note, that in general rewriting history in this way is a hindrance to the review process and this
186should only be done to correct a DCO mistake.
187
188## Triggering CI re-run without making changes
189
190Sometimes tasks will be stuck in CI and won't be marked as failed, which means
191the above command won't work. Should this happen, pushing an empty commit should
192re-run all the CI tasks. Consider adding an alias into your `.gitconfig` file:
193
194```
195[alias]
196 kick-ci = !"git commit -s --allow-empty -m 'Kick CI' && git push"
197```
198
199Once you add this alias you can issue the command `git kick-ci` and the PR
200will be sent back for a retest.View as plain text