blob: 01857da9700b3fe963383919e7eee3c9d883e244 [file] [log] [blame]
Marian Harbachebeb1542019-12-13 10:42:46 +01001:linkattrs:
Yuxuan 'fishy' Wang61698b12013-12-20 12:55:51 -08002= Gerrit Code Review - Contributing
Martin Fick081fa512011-08-12 11:22:45 -06003
Edwin Kempin21186702019-04-24 10:07:09 +02004[[cla]]
5== Contributor License Agreement
Martin Fick081fa512011-08-12 11:22:45 -06006
Edwin Kempin21186702019-04-24 10:07:09 +02007In order to contribute to Gerrit a link:dev-cla.html[Contributor
Marco Miller50107212020-04-21 15:42:55 -04008License Agreement,role=external,window=_blank] must be completed before
9contributions are accepted.
Edwin Kempin21186702019-04-24 10:07:09 +020010
Edwin Kempin099a5ec2019-04-25 16:15:14 +020011[[contribution-processes]]
12== Contribution Processes
13
14The Gerrit project offers two contribution processes:
15
16* link:#lightweight-contribution-process[Lightweight Contribution
17 Process]
18* link:#design-driven-contribution-process[Design-Driven Contribution
19 Process]
20
21The lightweight contribution process has little overhead and is best
22suited for small contributions (documentation updates, bug fixes, small
23features). Contributions are pushed as changes and
Marco Miller50107212020-04-21 15:42:55 -040024link:dev-roles.html#maintainer[maintainers,role=external,window=_blank]
25review them adhoc.
Edwin Kempin099a5ec2019-04-25 16:15:14 +020026
27For large/complex features, it is required to follow the
28link:#design-driven-contribution-process[design-driven contribution
29process] and specify the feature in a link:dev-design-docs.html[design
Marco Miller50107212020-04-21 15:42:55 -040030doc,role=external,window=_blank] before starting with the implementation.
Edwin Kempin099a5ec2019-04-25 16:15:14 +020031
Marco Miller50107212020-04-21 15:42:55 -040032If link:dev-roles.html#contributor[contributors,role=external,window=_blank]
33choose the lightweight contribution process and during the review it turns out
Edwin Kempin099a5ec2019-04-25 16:15:14 +020034that the feature is too large or complex,
Marco Miller50107212020-04-21 15:42:55 -040035link:dev-roles.html#maintainer[maintainers,role=external,window=_blank] can
36require to follow the design-driven contribution process instead.
Edwin Kempin099a5ec2019-04-25 16:15:14 +020037
38If you are in doubt which process is right for you, consult the
Marian Harbach34253372019-12-10 18:01:31 +010039link:https://groups.google.com/d/forum/repo-discuss[repo-discuss,role=external,window=_blank]
Edwin Kempin099a5ec2019-04-25 16:15:14 +020040mailing list.
41
42These contribution processes apply to everyone who contributes code to
43the Gerrit project, including link:dev-roles.html#maintainer[
Marco Miller50107212020-04-21 15:42:55 -040044maintainers,role=external,window=_blank]. When reading this document, keep in
45mind that maintainers are also contributors when they contribute code.
Edwin Kempin099a5ec2019-04-25 16:15:14 +020046
Edwin Kempinf13dfa92019-05-02 13:55:43 +020047If a new feature is large or complex, it is often difficult to find a
48maintainer who can take the time that is needed for a thorough review,
49and who can help with getting the changes submitted. To avoid that this
50results in unpredictable long waiting times during code review,
51contributors can ask for link:#mentorship[mentor support]. A mentor
52helps with timely code reviews and technical guidance. Doing the
53implementation is still the responsibility of the contributor.
54
Edwin Kempin099a5ec2019-04-25 16:15:14 +020055[[comparison]]
56=== Quick Comparison
57
58[options="header"]
59|======================
60| |Lightweight Contribution Process|Design-Driven Contribution Process
61|Overhead|low (write good commit message, address review comments)|
Marco Miller50107212020-04-21 15:42:55 -040062high (write link:dev-design-docs.html[design doc,role=external,window=_blank]
63and get it approved)
Edwin Kempin099a5ec2019-04-25 16:15:14 +020064|Technical Guidance|by reviewer|during the design review and by
Edwin Kempinf13dfa92019-05-02 13:55:43 +020065reviewer/mentor
66|Review |adhoc (when reviewer is available)|by a dedicated mentor (if
67a link:#mentorship[mentor] was assigned)
Edwin Kempin099a5ec2019-04-25 16:15:14 +020068|Caveats |features may get vetoed after the implementation was already
69done, maintainers may make the design-driven contribution process
70required if a change gets too complex/large|design doc must stay open
Edwin Kempinf13dfa92019-05-02 13:55:43 +020071for a minimum of 10 calendar days, a mentor may not be available
72immediately
Edwin Kempin099a5ec2019-04-25 16:15:14 +020073|Applicable to|documentation updates, bug fixes, small features|
74large/complex features
75|======================
76
77[[lightweight-contribution-process]]
78=== Lightweight Contribution Process
79
80The lightweight contribution process has little overhead and is best
81suited for small contributions (documentation updates, bug fixes, small
82features). For large/complex features the
83link:#design-driven-contribution-process[design-driven contribution
84process] is required.
Edwin Kempin21186702019-04-24 10:07:09 +020085
Martin Fick081fa512011-08-12 11:22:45 -060086As Gerrit is a code review tool, naturally contributions will
87be reviewed before they will get submitted to the code base. To
88start your contribution, please make a git commit and upload it
Edwin Kempin099a5ec2019-04-25 16:15:14 +020089for review to the link:https://gerrit-review.googlesource.com/[
Marco Miller50107212020-04-21 15:42:55 -040090gerrit-review.googlesource.com,role=external,window=_blank] Gerrit server. To
91help speed up the review of your change, review these link:dev-crafting-changes.html[
92guidelines,role=external,window=_blank] before submitting your change. You can
93view the pending Gerrit contributions and their statuses
Marian Harbach34253372019-12-10 18:01:31 +010094link:https://gerrit-review.googlesource.com/#/q/status:open+project:gerrit[here,role=external,window=_blank].
Martin Fick081fa512011-08-12 11:22:45 -060095
96Depending on the size of that list it might take a while for
97your change to get reviewed. Naturally there are fewer
Marco Miller50107212020-04-21 15:42:55 -040098link:dev-roles.html#maintainer[maintainers,role=external,window=_blank], that
99can approve changes, than link:dev-roles.html#contributor[contributors,role=external,window=_blank];
100so anything that you can do to ensure that your contribution will undergo fewer
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200101revisions will speed up the contribution process. This includes
102helping out reviewing other people's changes to relieve the load from
103the maintainers. Even if you are not familiar with Gerrit's internals,
104it would be of great help if you can download, try out, and comment on
105new features. If it works as advertised, say so, and if you have the
106privileges to do so, go ahead and give it a `+1 Verified`. If you
107would find the feature useful, say so and give it a `+1 Code Review`.
Martin Fick081fa512011-08-12 11:22:45 -0600108
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200109And finally, the quicker you respond to the comments of your reviewers,
110the quicker your change might get merged! Try to reply to every
111comment after submitting your new patch, particularly if you decided
112against making the suggested change. Reviewers don't want to seem like
113nags and pester you if you haven't replied or made a fix, so it helps
114them know if you missed it or decided against it.
115
Han-Wen Nienhuys0ad68fb2021-02-04 16:33:28 +0100116Features or API extensions, even if they are small, will incur
117long-time maintenance and support burden, so they should be left
118pending for at least 24 hours to give maintainers in all timezones a
119chance to evaluate.
120
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200121[[design-driven-contribution-process]]
122=== Design-driven Contribution Process
123
124The design-driven contribution process applies to large/complex
125features.
126
127For large/complex features it is important to:
128
129* agree on the functionality and scope before spending too much time
130 on the implementation
131* ensure that they are in line with Gerrit's project scope and vision
132* ensure that they are well aligned with other features
133* think about possibilities how the feature could be evolved over time
134
135This is why for large/complex features it is required to describe the
Marco Miller50107212020-04-21 15:42:55 -0400136feature in a link:dev-design-docs.html[design doc,role=external,window=_blank]
137and get it approved by the
138link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank],
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200139before starting the implementation.
140
141The design-driven contribution process has the following steps:
142
Marco Miller50107212020-04-21 15:42:55 -0400143* A link:dev-roles.html#contributor[contributor,role=external,window=_blank]
144 link:dev-design-docs.html#propose[proposes,role=external,window=_blank] a new
145 feature by uploading a change with a
146 link:dev-design-docs.html[design doc,role=external,window=_blank].
147* The design doc is link:dev-design-docs.html#review[reviewed,role=external,window=_blank]
148 by interested parties from the community. The design review is public
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200149 and everyone can comment and raise concerns.
150* Design docs should stay open for a minimum of 10 calendar days so
151 that everyone has a fair chance to join the review.
Edwin Kempinc507dbb2020-06-08 11:53:56 +0200152* Within 30 calendar days the contributor should hear back from the
Marco Miller50107212020-04-21 15:42:55 -0400153 link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank]
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200154 whether the proposed feature is in scope of the project and if it can
155 be accepted.
156* To be submitted, the design doc needs to be approved by the
Marco Miller50107212020-04-21 15:42:55 -0400157 link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank].
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200158* After the design was approved, the implementation is done by pushing
159 changes for review, see link:#lightweight-contribution-process[
160 lightweight contribution process]. Changes that are associated with
161 a design should all share a common hashtag. The contributor is the
162 main driver of the implementation and responsible that it is done.
163 Others from the Gerrit community are usually much welcome to help
164 with the implementation.
165
166In order to be accepted/submitted, it is not necessary that the design
167doc fully specifies all the details, but the idea of the feature and
168how it fits into Gerrit should be sufficiently clear (judged by the
169steering committee). Contributors are expected to keep the design doc
170updated and fill in gaps while they go forward with the implementation.
171We expect that implementing the feature and updating the design doc
172will be an iterative process.
173
174While the design doc is still in review, contributors may already start
175with the implementation (e.g. do some prototyping to demonstrate parts
176of the proposed design), but those changes should not be submitted
177while the design wasn't approved yet.
178
179By approving a design, the steering committee commits to:
180
181* Accepting the feature when it is implemented.
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200182* Supporting the feature by assigning a link:dev-roles.html#mentor[
Marco Miller50107212020-04-21 15:42:55 -0400183 mentor,role=external,window=_blank] (if requested, see link:#mentorship[mentorship]).
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200184
185If the implementation of a feature gets stuck and it's unclear whether
186the feature gets fully done, it should be discussed with the steering
187committee how to proceed. If the contributor cannot commit to finish
188the implementation and no other contributor can take over, changes that
189have already been submitted for the feature might get reverted so that
190there is no unused or half-finished code in the code base.
191
192For contributors, the design-driven contribution process has the
193following advantages:
194
195* By writing a design doc, the feature gets more attention. During the
196 design review, feedback from various sides can be collected, which
197 likely leads to improvements of the feature.
198* Once a design was approved by the
Marco Miller50107212020-04-21 15:42:55 -0400199 link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank],
200 the contributor can be almost certain that the feature will be accepted.
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200201 Hence, there is only a low risk to invest into implementing a feature
202 and see it being rejected later during the code review, as it can
203 happen with the lightweight contribution process.
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200204* The contributor can link:#mentorship[get a dedicated mentor assigned]
205 who provides timely reviews and serves as a contact person for
206 technical questions and discussing details of the design.
207
208[[mentorship]]
209== Mentorship
210
Marco Miller50107212020-04-21 15:42:55 -0400211For features for which a link:dev-design-docs.html[design,role=external,window=_blank]
212has been approved (see link:#design-driven-contribution-process[design-driven
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200213contribution process]), contributors can gain the support of a mentor
214if they are committed to implement the feature.
215
Marco Miller50107212020-04-21 15:42:55 -0400216A link:dev-roles.html#mentor[mentor,role=external,window=_blank] helps with:
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200217
218* doing timely reviews
219* providing technical guidance during code reviews
220* discussing details of the design
221* ensuring that the quality standards are met (well documented,
222 sufficient test coverage, backwards compatible etc.)
223
224A feature can have more than one mentor. To be able to deliver the
225promised support, at least one of the mentors must be a
Marco Miller50107212020-04-21 15:42:55 -0400226link:dev-roles.html#maintainer[maintainer,role=external,window=_blank].
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200227
228Mentors are assigned by the link:dev-processes.html#steering-committee[
Marco Miller50107212020-04-21 15:42:55 -0400229steering committee,role=external,window=_blank]. To gain a mentor, ask for a
230mentor in the link:dev-design-doc-template.html#implementation-plan[Implementation
231Plan,role=external,window=_blank] section of the design doc or ask the steering
232committee after the design has been approved.
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200233
234Mentors may not be available immediately. In this case, the steering
235committee should include the approved feature into the roadmap or
236prioritize it in the backlog. This way, it is transparent for the
237contributor when they can expect to be able to work on the feature with
238mentor support.
239
240Once the implementation phase starts, the contributor is expected to do
241the implementation in a timely manner.
242
243For every mentorship, the end must be clearly defined. The design doc
244must specify:
245
246* a maximum time frame for the mentorship, after which the mentorship
247 automatically ends, even if the feature is not done yet
248* done criteria that define when the feature is done and the mentorship
249 ends
250
251If a feature is not finished in time, it should be discussed with the
252steering committee how to proceed. If the contributor cannot commit to
253finish the implementation in time and no other contributor can take
254over, changes that have already been submitted for the feature might
255get reverted so that there is no unused or half-finished code in the
256code base.
Martin Fick081fa512011-08-12 11:22:45 -0600257
Patrick Hieselc49eea22020-03-24 10:04:18 +0100258[[esc-dd-evaluation]]
259== How the ESC evaluates design documents
260This section describes how the ESC evaluates design documents. It’s
261meant as a guideline rather than being prescriptive for both ESC
262members and contributors.
263
264=== General Process
265As part of the design process, the ESC makes a final decision if a
266design gets to be implemented. If there are multiple alternative
267solutions, the ESC will decide which solution can be implemented.
268
269The ESC should wait until all contributors had the chance to
270voice their opinion in review comments or by proposing alternative
271solutions. Due to the infrequent ESC meetings (every 2-4 weeks)
272the ESC might discuss documents in cases where the discussion is
273already advanced far enough, but not make a decision yet. In this
274case, contributors can still voice concerns or discuss alternatives.
275The decision can be at the next meeting or via email in between
276meetings.
277
278=== Evaluation
279Product/Vision fit
Marco Miller1e543582020-04-07 16:58:59 -0400280
Patrick Hieselc49eea22020-03-24 10:04:18 +0100281Q: `Do we believe this feature belongs to Gerrit Code Review use-cases?`
Marco Miller1e543582020-04-07 16:58:59 -0400282
Patrick Hieselc49eea22020-03-24 10:04:18 +0100283* Yes: Customizable dashboards
284* No: UI for managing an LDAP server
285
286Q: `Is the proposed solution aligned with Gerrit’s vision?`
Marco Miller1e543582020-04-07 16:58:59 -0400287
Patrick Hieselc49eea22020-03-24 10:04:18 +0100288* Yes: Showing comments of older patch sets in newer patch sets to
289 keep track (core code review)
290* No: Implement a bug tracker in Gerrit (not core code review).
291
292=== Impact
293Q: `Will the new feature have a measurable, positive impact?`
Marco Miller1e543582020-04-07 16:58:59 -0400294
Patrick Hieselc49eea22020-03-24 10:04:18 +0100295* Yes: Increased productivity, faster/smoother workflow, etc.
296* Yes: Better latency, more reliable system.
297* No: Unclear impact or lacking metrics to measure.
298
299=== Complexity
300Q: `Can we support/maintain this feature once it is in Gerrit?`
Marco Miller1e543582020-04-07 16:58:59 -0400301
Patrick Hieselc49eea22020-03-24 10:04:18 +0100302* Yes: Code will fit into codebase, be well tested, easy to
303 understand.
304* No: Will add code or a workflow that is hard to understand
305 and easy to misinterpret.
Marco Miller1e543582020-04-07 16:58:59 -0400306
Patrick Hieselc49eea22020-03-24 10:04:18 +0100307Q: `Is the proposed feature or rework adding unnecessary complexity?`
Marco Miller1e543582020-04-07 16:58:59 -0400308
Patrick Hieselc49eea22020-03-24 10:04:18 +0100309* Yes: Adding a dependency on a well-supported library.
310* No: Adding a dependency on a library that is not widely used
311 or not actively maintained.
312
313=== Core vs. Plugin decision
314Q: `Would this fit better in a plugin?`
Marco Miller1e543582020-04-07 16:58:59 -0400315
Patrick Hieselc49eea22020-03-24 10:04:18 +0100316* Yes:The proposed feature or rework is an implementation (e.g. Lucene
317 is an index implementation) of a generic concept that others
318 might want to implement differently.
319* Yes: The proposed feature or rework is very specific to a custom setup.
320* No: The proposed feature or rework is applicable to a wider user
321 base.
322* No: The proposed feature or rework is a `core code review feature`.
323
324=== Commitment
325Q: `Is someone willing to implement it?` (this question is
326especially important when reviewers propose alternative designs
327to the author’s own solution).
Marco Miller1e543582020-04-07 16:58:59 -0400328
Patrick Hieselc49eea22020-03-24 10:04:18 +0100329* Yes: The author or someone else commits to implementing the
330 proposed solution.
331* Yes: If a mentorship is required, a mentor is willing to help.
332* No: Unclear ownership, mentorship or implementation plan.
333
334=== Community
335Q: `If in doubt, is there a substantial benefit to a long-standing
336community member with many users?`
Marco Miller1e543582020-04-07 16:58:59 -0400337
Patrick Hieselc49eea22020-03-24 10:04:18 +0100338* The community shapes the future of Gerrit as a product. In
Marco Millerbe6a9d32020-04-07 16:33:32 -0400339 cases of doubt, the ESC can be more generous with long-standing
340 community members compared to `drive-by` contributions.
Patrick Hiesel078ad842020-03-24 10:04:18 +0100341
Martin Fick081fa512011-08-12 11:22:45 -0600342GERRIT
343------
344Part of link:index.html[Gerrit Code Review]
Yuxuan 'fishy' Wang99cb68d2013-10-31 17:26:00 -0700345
346SEARCHBOX
347---------