blob: 477641bee8977d18c3ed8ed0670374879bb6ee10 [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
116[[design-driven-contribution-process]]
117=== Design-driven Contribution Process
118
119The design-driven contribution process applies to large/complex
120features.
121
122For large/complex features it is important to:
123
124* agree on the functionality and scope before spending too much time
125 on the implementation
126* ensure that they are in line with Gerrit's project scope and vision
127* ensure that they are well aligned with other features
128* think about possibilities how the feature could be evolved over time
129
130This is why for large/complex features it is required to describe the
Marco Miller50107212020-04-21 15:42:55 -0400131feature in a link:dev-design-docs.html[design doc,role=external,window=_blank]
132and get it approved by the
133link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank],
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200134before starting the implementation.
135
136The design-driven contribution process has the following steps:
137
Marco Miller50107212020-04-21 15:42:55 -0400138* A link:dev-roles.html#contributor[contributor,role=external,window=_blank]
139 link:dev-design-docs.html#propose[proposes,role=external,window=_blank] a new
140 feature by uploading a change with a
141 link:dev-design-docs.html[design doc,role=external,window=_blank].
142* The design doc is link:dev-design-docs.html#review[reviewed,role=external,window=_blank]
143 by interested parties from the community. The design review is public
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200144 and everyone can comment and raise concerns.
145* Design docs should stay open for a minimum of 10 calendar days so
146 that everyone has a fair chance to join the review.
Edwin Kempinc507dbb2020-06-08 11:53:56 +0200147* Within 30 calendar days the contributor should hear back from the
Marco Miller50107212020-04-21 15:42:55 -0400148 link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank]
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200149 whether the proposed feature is in scope of the project and if it can
150 be accepted.
151* To be submitted, the design doc needs to be approved by the
Marco Miller50107212020-04-21 15:42:55 -0400152 link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank].
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200153* After the design was approved, the implementation is done by pushing
154 changes for review, see link:#lightweight-contribution-process[
155 lightweight contribution process]. Changes that are associated with
156 a design should all share a common hashtag. The contributor is the
157 main driver of the implementation and responsible that it is done.
158 Others from the Gerrit community are usually much welcome to help
159 with the implementation.
160
161In order to be accepted/submitted, it is not necessary that the design
162doc fully specifies all the details, but the idea of the feature and
163how it fits into Gerrit should be sufficiently clear (judged by the
164steering committee). Contributors are expected to keep the design doc
165updated and fill in gaps while they go forward with the implementation.
166We expect that implementing the feature and updating the design doc
167will be an iterative process.
168
169While the design doc is still in review, contributors may already start
170with the implementation (e.g. do some prototyping to demonstrate parts
171of the proposed design), but those changes should not be submitted
172while the design wasn't approved yet.
173
174By approving a design, the steering committee commits to:
175
176* Accepting the feature when it is implemented.
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200177* Supporting the feature by assigning a link:dev-roles.html#mentor[
Marco Miller50107212020-04-21 15:42:55 -0400178 mentor,role=external,window=_blank] (if requested, see link:#mentorship[mentorship]).
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200179
180If the implementation of a feature gets stuck and it's unclear whether
181the feature gets fully done, it should be discussed with the steering
182committee how to proceed. If the contributor cannot commit to finish
183the implementation and no other contributor can take over, changes that
184have already been submitted for the feature might get reverted so that
185there is no unused or half-finished code in the code base.
186
187For contributors, the design-driven contribution process has the
188following advantages:
189
190* By writing a design doc, the feature gets more attention. During the
191 design review, feedback from various sides can be collected, which
192 likely leads to improvements of the feature.
193* Once a design was approved by the
Marco Miller50107212020-04-21 15:42:55 -0400194 link:dev-processes.html#steering-committee[steering committee,role=external,window=_blank],
195 the contributor can be almost certain that the feature will be accepted.
Edwin Kempin099a5ec2019-04-25 16:15:14 +0200196 Hence, there is only a low risk to invest into implementing a feature
197 and see it being rejected later during the code review, as it can
198 happen with the lightweight contribution process.
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200199* The contributor can link:#mentorship[get a dedicated mentor assigned]
200 who provides timely reviews and serves as a contact person for
201 technical questions and discussing details of the design.
202
203[[mentorship]]
204== Mentorship
205
Marco Miller50107212020-04-21 15:42:55 -0400206For features for which a link:dev-design-docs.html[design,role=external,window=_blank]
207has been approved (see link:#design-driven-contribution-process[design-driven
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200208contribution process]), contributors can gain the support of a mentor
209if they are committed to implement the feature.
210
Marco Miller50107212020-04-21 15:42:55 -0400211A link:dev-roles.html#mentor[mentor,role=external,window=_blank] helps with:
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200212
213* doing timely reviews
214* providing technical guidance during code reviews
215* discussing details of the design
216* ensuring that the quality standards are met (well documented,
217 sufficient test coverage, backwards compatible etc.)
218
219A feature can have more than one mentor. To be able to deliver the
220promised support, at least one of the mentors must be a
Marco Miller50107212020-04-21 15:42:55 -0400221link:dev-roles.html#maintainer[maintainer,role=external,window=_blank].
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200222
223Mentors are assigned by the link:dev-processes.html#steering-committee[
Marco Miller50107212020-04-21 15:42:55 -0400224steering committee,role=external,window=_blank]. To gain a mentor, ask for a
225mentor in the link:dev-design-doc-template.html#implementation-plan[Implementation
226Plan,role=external,window=_blank] section of the design doc or ask the steering
227committee after the design has been approved.
Edwin Kempinf13dfa92019-05-02 13:55:43 +0200228
229Mentors may not be available immediately. In this case, the steering
230committee should include the approved feature into the roadmap or
231prioritize it in the backlog. This way, it is transparent for the
232contributor when they can expect to be able to work on the feature with
233mentor support.
234
235Once the implementation phase starts, the contributor is expected to do
236the implementation in a timely manner.
237
238For every mentorship, the end must be clearly defined. The design doc
239must specify:
240
241* a maximum time frame for the mentorship, after which the mentorship
242 automatically ends, even if the feature is not done yet
243* done criteria that define when the feature is done and the mentorship
244 ends
245
246If a feature is not finished in time, it should be discussed with the
247steering committee how to proceed. If the contributor cannot commit to
248finish the implementation in time and no other contributor can take
249over, changes that have already been submitted for the feature might
250get reverted so that there is no unused or half-finished code in the
251code base.
Martin Fick081fa512011-08-12 11:22:45 -0600252
Patrick Hieselc49eea22020-03-24 10:04:18 +0100253[[esc-dd-evaluation]]
254== How the ESC evaluates design documents
255This section describes how the ESC evaluates design documents. It’s
256meant as a guideline rather than being prescriptive for both ESC
257members and contributors.
258
259=== General Process
260As part of the design process, the ESC makes a final decision if a
261design gets to be implemented. If there are multiple alternative
262solutions, the ESC will decide which solution can be implemented.
263
264The ESC should wait until all contributors had the chance to
265voice their opinion in review comments or by proposing alternative
266solutions. Due to the infrequent ESC meetings (every 2-4 weeks)
267the ESC might discuss documents in cases where the discussion is
268already advanced far enough, but not make a decision yet. In this
269case, contributors can still voice concerns or discuss alternatives.
270The decision can be at the next meeting or via email in between
271meetings.
272
273=== Evaluation
274Product/Vision fit
Marco Miller1e543582020-04-07 16:58:59 -0400275
Patrick Hieselc49eea22020-03-24 10:04:18 +0100276Q: `Do we believe this feature belongs to Gerrit Code Review use-cases?`
Marco Miller1e543582020-04-07 16:58:59 -0400277
Patrick Hieselc49eea22020-03-24 10:04:18 +0100278* Yes: Customizable dashboards
279* No: UI for managing an LDAP server
280
281Q: `Is the proposed solution aligned with Gerrit’s vision?`
Marco Miller1e543582020-04-07 16:58:59 -0400282
Patrick Hieselc49eea22020-03-24 10:04:18 +0100283* Yes: Showing comments of older patch sets in newer patch sets to
284 keep track (core code review)
285* No: Implement a bug tracker in Gerrit (not core code review).
286
287=== Impact
288Q: `Will the new feature have a measurable, positive impact?`
Marco Miller1e543582020-04-07 16:58:59 -0400289
Patrick Hieselc49eea22020-03-24 10:04:18 +0100290* Yes: Increased productivity, faster/smoother workflow, etc.
291* Yes: Better latency, more reliable system.
292* No: Unclear impact or lacking metrics to measure.
293
294=== Complexity
295Q: `Can we support/maintain this feature once it is in Gerrit?`
Marco Miller1e543582020-04-07 16:58:59 -0400296
Patrick Hieselc49eea22020-03-24 10:04:18 +0100297* Yes: Code will fit into codebase, be well tested, easy to
298 understand.
299* No: Will add code or a workflow that is hard to understand
300 and easy to misinterpret.
Marco Miller1e543582020-04-07 16:58:59 -0400301
Patrick Hieselc49eea22020-03-24 10:04:18 +0100302Q: `Is the proposed feature or rework adding unnecessary complexity?`
Marco Miller1e543582020-04-07 16:58:59 -0400303
Patrick Hieselc49eea22020-03-24 10:04:18 +0100304* Yes: Adding a dependency on a well-supported library.
305* No: Adding a dependency on a library that is not widely used
306 or not actively maintained.
307
308=== Core vs. Plugin decision
309Q: `Would this fit better in a plugin?`
Marco Miller1e543582020-04-07 16:58:59 -0400310
Patrick Hieselc49eea22020-03-24 10:04:18 +0100311* Yes:The proposed feature or rework is an implementation (e.g. Lucene
312 is an index implementation) of a generic concept that others
313 might want to implement differently.
314* Yes: The proposed feature or rework is very specific to a custom setup.
315* No: The proposed feature or rework is applicable to a wider user
316 base.
317* No: The proposed feature or rework is a `core code review feature`.
318
319=== Commitment
320Q: `Is someone willing to implement it?` (this question is
321especially important when reviewers propose alternative designs
322to the author’s own solution).
Marco Miller1e543582020-04-07 16:58:59 -0400323
Patrick Hieselc49eea22020-03-24 10:04:18 +0100324* Yes: The author or someone else commits to implementing the
325 proposed solution.
326* Yes: If a mentorship is required, a mentor is willing to help.
327* No: Unclear ownership, mentorship or implementation plan.
328
329=== Community
330Q: `If in doubt, is there a substantial benefit to a long-standing
331community member with many users?`
Marco Miller1e543582020-04-07 16:58:59 -0400332
Patrick Hieselc49eea22020-03-24 10:04:18 +0100333* The community shapes the future of Gerrit as a product. In
Marco Millerbe6a9d32020-04-07 16:33:32 -0400334 cases of doubt, the ESC can be more generous with long-standing
335 community members compared to `drive-by` contributions.
Patrick Hiesel078ad842020-03-24 10:04:18 +0100336
Martin Fick081fa512011-08-12 11:22:45 -0600337GERRIT
338------
339Part of link:index.html[Gerrit Code Review]
Yuxuan 'fishy' Wang99cb68d2013-10-31 17:26:00 -0700340
341SEARCHBOX
342---------