xref: /aosp_15_r20/external/flashrom/doc/dev_guide/development_guide.rst (revision 0d6140be3aa665ecc836e8907834fcd3e3b018fc)
1*0d6140beSAndroid Build Coastguard Worker=================
2*0d6140beSAndroid Build Coastguard WorkerDevelopment Guide
3*0d6140beSAndroid Build Coastguard Worker=================
4*0d6140beSAndroid Build Coastguard Worker
5*0d6140beSAndroid Build Coastguard WorkerWe welcome contributions from every human being, corporate entity or club.
6*0d6140beSAndroid Build Coastguard Worker
7*0d6140beSAndroid Build Coastguard WorkerThis document describes the rules and recommendations about the development, contribution and review processes.
8*0d6140beSAndroid Build Coastguard Worker
9*0d6140beSAndroid Build Coastguard WorkerIf you introduce new features (not flash chips, but stuff like partial
10*0d6140beSAndroid Build Coastguard Workerprogramming, support for new external programmers, voltage handling, etc)
11*0d6140beSAndroid Build Coastguard Workerplease **discuss your plans** on the :ref:`mailing list` first. That way, we
12*0d6140beSAndroid Build Coastguard Workercan avoid duplicated work and know about how flashrom internals need to be
13*0d6140beSAndroid Build Coastguard Workeradjusted and you avoid frustration if there is some disagreement about the
14*0d6140beSAndroid Build Coastguard Workerdesign.
15*0d6140beSAndroid Build Coastguard Worker
16*0d6140beSAndroid Build Coastguard WorkerYou can `look at the latest flashrom development efforts in Gerrit <https://review.coreboot.org/q/project:flashrom>`_.
17*0d6140beSAndroid Build Coastguard Worker
18*0d6140beSAndroid Build Coastguard WorkerSet up the git repository and dev environment
19*0d6140beSAndroid Build Coastguard Worker=============================================
20*0d6140beSAndroid Build Coastguard Worker
21*0d6140beSAndroid Build Coastguard Worker#. Clone git repository
22*0d6140beSAndroid Build Coastguard Worker
23*0d6140beSAndroid Build Coastguard Worker   * If using https: :code:`git clone "https://review.coreboot.org/flashrom"`
24*0d6140beSAndroid Build Coastguard Worker   * If using ssh: :code:`git clone "ssh://<gerrit_username>@review.coreboot.org:29418/flashrom"`
25*0d6140beSAndroid Build Coastguard Worker
26*0d6140beSAndroid Build Coastguard Worker#. Follow the build guidelines to install dependencies :doc:`building_from_source`
27*0d6140beSAndroid Build Coastguard Worker
28*0d6140beSAndroid Build Coastguard Worker#. Install Git hooks: :code:`./util/git-hooks/install.sh`
29*0d6140beSAndroid Build Coastguard Worker
30*0d6140beSAndroid Build Coastguard Worker#. Add upstream as a remote:
31*0d6140beSAndroid Build Coastguard Worker
32*0d6140beSAndroid Build Coastguard Worker   * If using https: :code:`git remote add -f upstream https://review.coreboot.org/flashrom`
33*0d6140beSAndroid Build Coastguard Worker   * If using ssh: :code:`git remote add -f upstream ssh://<gerrit_username>@review.coreboot.org:29418/flashrom`
34*0d6140beSAndroid Build Coastguard Worker
35*0d6140beSAndroid Build Coastguard Worker#. Check out a new local branch that tracks :code:`upstream/master`: :code:`git checkout -b <branch_name> upstream/master`
36*0d6140beSAndroid Build Coastguard Worker
37*0d6140beSAndroid Build Coastguard Worker#. Every patch is required to be signed-off (see also :ref:`sign-off`).
38*0d6140beSAndroid Build Coastguard Worker   Set up your ``user.name`` and ``user.email`` in git config, and don't forget
39*0d6140beSAndroid Build Coastguard Worker   to ``-s`` when creating a commit.
40*0d6140beSAndroid Build Coastguard Worker
41*0d6140beSAndroid Build Coastguard Worker#. See also build guidelines :doc:`building_from_source` and `git docs <https://git-scm.com/doc>`_
42*0d6140beSAndroid Build Coastguard Worker
43*0d6140beSAndroid Build Coastguard WorkerCreating your patch
44*0d6140beSAndroid Build Coastguard Worker===================
45*0d6140beSAndroid Build Coastguard Worker
46*0d6140beSAndroid Build Coastguard WorkerIn short, commit your changes with a descriptive message and remember to sign off
47*0d6140beSAndroid Build Coastguard Workeron the commit (``git commit -s``).
48*0d6140beSAndroid Build Coastguard Worker
49*0d6140beSAndroid Build Coastguard Worker.. _commit-message:
50*0d6140beSAndroid Build Coastguard Worker
51*0d6140beSAndroid Build Coastguard WorkerCommit message
52*0d6140beSAndroid Build Coastguard Worker--------------
53*0d6140beSAndroid Build Coastguard Worker
54*0d6140beSAndroid Build Coastguard WorkerCommit messages shall have the following format::
55*0d6140beSAndroid Build Coastguard Worker
56*0d6140beSAndroid Build Coastguard Worker    <component>: Short description (up to 72 characters)
57*0d6140beSAndroid Build Coastguard Worker
58*0d6140beSAndroid Build Coastguard Worker    This is a long description. Max width of each line in the description
59*0d6140beSAndroid Build Coastguard Worker    is 72 characters. It is separated from the summary by a blank line. You
60*0d6140beSAndroid Build Coastguard Worker    may skip the long description if the short description is sufficient,
61*0d6140beSAndroid Build Coastguard Worker    for example "flashchips: Add FOO25Q128" to add FOO25Q128 chip support.
62*0d6140beSAndroid Build Coastguard Worker
63*0d6140beSAndroid Build Coastguard Worker    You may have multiple paragraphs in the long description, but please
64*0d6140beSAndroid Build Coastguard Worker    do not write a novel here. For non-trivial changes you must explain
65*0d6140beSAndroid Build Coastguard Worker    what your patch does, why, and how it was tested.
66*0d6140beSAndroid Build Coastguard Worker
67*0d6140beSAndroid Build Coastguard Worker    Finally, follow the sign-off procedure to add your sign-off!
68*0d6140beSAndroid Build Coastguard Worker
69*0d6140beSAndroid Build Coastguard Worker    Signed-off-by: Your Name <your@domain>
70*0d6140beSAndroid Build Coastguard Worker
71*0d6140beSAndroid Build Coastguard WorkerCommit message should include:
72*0d6140beSAndroid Build Coastguard Worker
73*0d6140beSAndroid Build Coastguard Worker* Commit title
74*0d6140beSAndroid Build Coastguard Worker* Commit description: explain what the patch is doing, or what it is fixing.
75*0d6140beSAndroid Build Coastguard Worker* Testing information: how did you test the patch.
76*0d6140beSAndroid Build Coastguard Worker* Signed-off-by line (see below :ref:`sign-off`)
77*0d6140beSAndroid Build Coastguard Worker* If applicable, link to the ticket in the bugtracker `<https://ticket.coreboot.org/projects/flashrom>`_
78*0d6140beSAndroid Build Coastguard Worker* Change-Id for Gerrit. If commit message doesn't have Change-Id, you forgot to install git hooks.
79*0d6140beSAndroid Build Coastguard Worker
80*0d6140beSAndroid Build Coastguard Worker.. _sign-off:
81*0d6140beSAndroid Build Coastguard Worker
82*0d6140beSAndroid Build Coastguard WorkerSign-off procedure
83*0d6140beSAndroid Build Coastguard Worker^^^^^^^^^^^^^^^^^^
84*0d6140beSAndroid Build Coastguard Worker
85*0d6140beSAndroid Build Coastguard WorkerWe employ a similar sign-off procedure as the `Linux kernel developers
86*0d6140beSAndroid Build Coastguard Worker<http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html>`_
87*0d6140beSAndroid Build Coastguard Workerdo. Add a note such as
88*0d6140beSAndroid Build Coastguard Worker
89*0d6140beSAndroid Build Coastguard Worker:code:`Signed-off-by: Random J Developer <[email protected]>`
90*0d6140beSAndroid Build Coastguard Worker
91*0d6140beSAndroid Build Coastguard Workerto your email/patch if you agree with the Developer's Certificate of Origin 1.1
92*0d6140beSAndroid Build Coastguard Workerprinted below. Read `this post on the LKML
93*0d6140beSAndroid Build Coastguard Worker<https://lkml.org/lkml/2004/5/23/10>`_ for rationale (spoiler: SCO).
94*0d6140beSAndroid Build Coastguard Worker
95*0d6140beSAndroid Build Coastguard WorkerYou must use your known identity in the ``Signed-off-by`` line and in any
96*0d6140beSAndroid Build Coastguard Workercopyright notices you add. Anonymous patches lack provenance and cannot be
97*0d6140beSAndroid Build Coastguard Workercommitted!
98*0d6140beSAndroid Build Coastguard Worker
99*0d6140beSAndroid Build Coastguard WorkerDeveloper's Certificate of Origin 1.1
100*0d6140beSAndroid Build Coastguard Worker^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
101*0d6140beSAndroid Build Coastguard Worker
102*0d6140beSAndroid Build Coastguard Worker    By making a contribution to this project, I certify that:
103*0d6140beSAndroid Build Coastguard Worker
104*0d6140beSAndroid Build Coastguard Worker    (a) The contribution was created in whole or in part by me and I have
105*0d6140beSAndroid Build Coastguard Worker    the right to submit it under the open source license indicated in the file; or
106*0d6140beSAndroid Build Coastguard Worker
107*0d6140beSAndroid Build Coastguard Worker    (b) The contribution is based upon previous work that, to the best of my
108*0d6140beSAndroid Build Coastguard Worker    knowledge, is covered under an appropriate open source license and I have the
109*0d6140beSAndroid Build Coastguard Worker    right under that license to submit that work with modifications, whether created
110*0d6140beSAndroid Build Coastguard Worker    in whole or in part by me, under the same open source license (unless I am
111*0d6140beSAndroid Build Coastguard Worker    permitted to submit under a different license), as indicated in the file; or
112*0d6140beSAndroid Build Coastguard Worker
113*0d6140beSAndroid Build Coastguard Worker    (c) The contribution was provided directly to me by some other person who
114*0d6140beSAndroid Build Coastguard Worker    certified (a), (b) or (c) and I have not modified it; and
115*0d6140beSAndroid Build Coastguard Worker
116*0d6140beSAndroid Build Coastguard Worker    (d) In the case of each of (a), (b), or (c), I understand and agree that
117*0d6140beSAndroid Build Coastguard Worker    this project and the contribution are public and that a record of the contribution
118*0d6140beSAndroid Build Coastguard Worker    (including all personal information I submit with it, including my sign-off) is
119*0d6140beSAndroid Build Coastguard Worker    maintained indefinitely and may be redistributed consistent with this project or the
120*0d6140beSAndroid Build Coastguard Worker    open source license indicated in the file.
121*0d6140beSAndroid Build Coastguard Worker
122*0d6140beSAndroid Build Coastguard Worker.. note::
123*0d6140beSAndroid Build Coastguard Worker
124*0d6140beSAndroid Build Coastguard Worker   The `Developer's Certificate of Origin 1.1
125*0d6140beSAndroid Build Coastguard Worker   <http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html>`_
126*0d6140beSAndroid Build Coastguard Worker   is licensed under the terms of the `Creative Commons Attribution-ShareAlike
127*0d6140beSAndroid Build Coastguard Worker   2.5 License <http://creativecommons.org/licenses/by-sa/2.5/>`_.
128*0d6140beSAndroid Build Coastguard Worker
129*0d6140beSAndroid Build Coastguard WorkerCoding style
130*0d6140beSAndroid Build Coastguard Worker------------
131*0d6140beSAndroid Build Coastguard Worker
132*0d6140beSAndroid Build Coastguard WorkerFlashrom generally follows Linux kernel style:
133*0d6140beSAndroid Build Coastguard Workerhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst
134*0d6140beSAndroid Build Coastguard Worker
135*0d6140beSAndroid Build Coastguard WorkerThe notable exception is line length limit. Our guidelines are:
136*0d6140beSAndroid Build Coastguard Worker
137*0d6140beSAndroid Build Coastguard Worker* 80-columns soft limit for most code and comments. This is to encourage simple design and concise naming.
138*0d6140beSAndroid Build Coastguard Worker* 112-columns hard limit. Use this to reduce line breaks in cases where they
139*0d6140beSAndroid Build Coastguard Worker  harm grep-ability or overall readability, such as print statements and
140*0d6140beSAndroid Build Coastguard Worker  function signatures. Don't abuse this for long variable/function names or
141*0d6140beSAndroid Build Coastguard Worker  deep nesting.
142*0d6140beSAndroid Build Coastguard Worker* Tables are the only exception to the hard limit and may be as long as needed
143*0d6140beSAndroid Build Coastguard Worker  for practical purposes.
144*0d6140beSAndroid Build Coastguard Worker
145*0d6140beSAndroid Build Coastguard WorkerOur guidelines borrow heavily from `coreboot coding style
146*0d6140beSAndroid Build Coastguard Worker<https://doc.coreboot.org/contributing/coding_style.html>`_ and `coreboot Gerrit
147*0d6140beSAndroid Build Coastguard Workerguidelines <https://doc.coreboot.org/contributing/gerrit_guidelines.html>`_,
148*0d6140beSAndroid Build Coastguard Workerand most of them apply to flashrom as well. The really important part is about
149*0d6140beSAndroid Build Coastguard Workerthe :ref:`sign-off procedure <sign-off>`.
150*0d6140beSAndroid Build Coastguard Worker
151*0d6140beSAndroid Build Coastguard WorkerWe try to **reuse as much code as possible** and create new files only if
152*0d6140beSAndroid Build Coastguard Workerabsolutely needed, so if you find a function somewhere in the tree which
153*0d6140beSAndroid Build Coastguard Workeralready does what you want, please use it.
154*0d6140beSAndroid Build Coastguard Worker
155*0d6140beSAndroid Build Coastguard WorkerTesting a patch
156*0d6140beSAndroid Build Coastguard Worker---------------
157*0d6140beSAndroid Build Coastguard Worker
158*0d6140beSAndroid Build Coastguard WorkerWe expect the patch to be appropriately tested by the patch owner.
159*0d6140beSAndroid Build Coastguard WorkerPlease add the testing information in commit message, for example that could be some of these:
160*0d6140beSAndroid Build Coastguard Workerprogrammer you were using, programmer params, chip, OS, operations you were running
161*0d6140beSAndroid Build Coastguard Worker(read/write/erase/verify), and anything else that is relevant.
162*0d6140beSAndroid Build Coastguard Worker
163*0d6140beSAndroid Build Coastguard Worker.. _working-with-gerrit:
164*0d6140beSAndroid Build Coastguard Worker
165*0d6140beSAndroid Build Coastguard WorkerWorking with Gerrit
166*0d6140beSAndroid Build Coastguard Worker===================
167*0d6140beSAndroid Build Coastguard Worker
168*0d6140beSAndroid Build Coastguard WorkerAll of the patches and code reviews need to go via
169*0d6140beSAndroid Build Coastguard Worker`Gerrit on review.coreboot.org <https://review.coreboot.org/#/q/project:flashrom>`_.
170*0d6140beSAndroid Build Coastguard WorkerWhile it is technically possible to send a patch to the mailing list, that patch
171*0d6140beSAndroid Build Coastguard Workerstill needs to be pushed to Gerrit by someone. We treat patches on the mailing list as a very
172*0d6140beSAndroid Build Coastguard Workerexceptional situation. Normal process is to push a patch to Gerrit.
173*0d6140beSAndroid Build Coastguard WorkerPlease read below for instructions and check `official Gerrit documentation <https://gerrit-review.googlesource.com/Documentation/>`_.
174*0d6140beSAndroid Build Coastguard Worker
175*0d6140beSAndroid Build Coastguard WorkerCreating an account
176*0d6140beSAndroid Build Coastguard Worker---------------------
177*0d6140beSAndroid Build Coastguard Worker
178*0d6140beSAndroid Build Coastguard Worker#. Go to https://review.coreboot.org/login and sign in using the credentials of
179*0d6140beSAndroid Build Coastguard Worker   your choice.
180*0d6140beSAndroid Build Coastguard Worker#. Edit your settings by clicking on the gear icon in the upper right corner.
181*0d6140beSAndroid Build Coastguard Worker#. Set your Gerrit username (this may be the different from the username of an
182*0d6140beSAndroid Build Coastguard Worker   external account you log in with).
183*0d6140beSAndroid Build Coastguard Worker#. Add an e-mail address so that Gerrit can send notifications to you about
184*0d6140beSAndroid Build Coastguard Worker   your patch.
185*0d6140beSAndroid Build Coastguard Worker#. Upload an SSH public key, or click the button to generate an HTTPS password.
186*0d6140beSAndroid Build Coastguard Worker#. After account created, set either "Full name" or "Display name", it is used by Gerrit
187*0d6140beSAndroid Build Coastguard Worker   for code review emails.
188*0d6140beSAndroid Build Coastguard Worker
189*0d6140beSAndroid Build Coastguard Worker.. _pushing-a-patch:
190*0d6140beSAndroid Build Coastguard Worker
191*0d6140beSAndroid Build Coastguard WorkerPushing a patch
192*0d6140beSAndroid Build Coastguard Worker---------------
193*0d6140beSAndroid Build Coastguard Worker
194*0d6140beSAndroid Build Coastguard WorkerBefore pushing a patch, make sure it builds on your environment and all unit tests pass (see :doc:`building_from_source`).
195*0d6140beSAndroid Build Coastguard Worker
196*0d6140beSAndroid Build Coastguard WorkerTo push patch to Gerrit, use the follow command: :code:`git push upstream HEAD:refs/for/main`.
197*0d6140beSAndroid Build Coastguard Worker
198*0d6140beSAndroid Build Coastguard Worker* If using HTTPS you will be prompted for the username and password you
199*0d6140beSAndroid Build Coastguard Worker  set in the Gerrit UI.
200*0d6140beSAndroid Build Coastguard Worker* If successful, the Gerrit URL for your patch will be shown in the output.
201*0d6140beSAndroid Build Coastguard Worker
202*0d6140beSAndroid Build Coastguard WorkerThere is an option to add a topic to the patch. For one-off standalone patches this
203*0d6140beSAndroid Build Coastguard Workeris not necessary. However if your patch is a part of a larger effort, especially if the
204*0d6140beSAndroid Build Coastguard Workerwork involves multiple contributors, it can be useful to mark that the patch belongs
205*0d6140beSAndroid Build Coastguard Workerto a certain topic.
206*0d6140beSAndroid Build Coastguard Worker
207*0d6140beSAndroid Build Coastguard WorkerAdding a topic makes it easy to search "all the patches by the topic", even if the patches
208*0d6140beSAndroid Build Coastguard Workerhave been authored by multiple people.
209*0d6140beSAndroid Build Coastguard Worker
210*0d6140beSAndroid Build Coastguard WorkerTo add a topic, push with the command: :code:`git push upstream HEAD:refs/for/master%topic=example_topic`.
211*0d6140beSAndroid Build Coastguard WorkerAlternatively, you can add a topic from a Gerrit UI after the patch in pushed
212*0d6140beSAndroid Build Coastguard Worker(on the top-left section) of patch UI.
213*0d6140beSAndroid Build Coastguard Worker
214*0d6140beSAndroid Build Coastguard WorkerChecking the CI
215*0d6140beSAndroid Build Coastguard Worker---------------
216*0d6140beSAndroid Build Coastguard Worker
217*0d6140beSAndroid Build Coastguard WorkerEvery patch needs to get a ``Verified +1`` label, typically from Jenkins. Once the patch is pushed
218*0d6140beSAndroid Build Coastguard Workerto Gerrit, Jenkins is added automatically and runs its build script. The script builds the patch with
219*0d6140beSAndroid Build Coastguard Workervarious config options, and runs unit tests (for more details see source code of ``test_build.sh``).
220*0d6140beSAndroid Build Coastguard WorkerThen, Jenkins gives the patch ``+1`` or ``-1`` vote, indicating success or fail.
221*0d6140beSAndroid Build Coastguard Worker
222*0d6140beSAndroid Build Coastguard WorkerIn case of failure, follow Jenkins link (which it adds as a comment to the patch), open Console output,
223*0d6140beSAndroid Build Coastguard Workerfind the error and try to fix it.
224*0d6140beSAndroid Build Coastguard Worker
225*0d6140beSAndroid Build Coastguard WorkerIn addition to building and running unit tests, Jenkins also runs a scan-build over the patch. Ideally
226*0d6140beSAndroid Build Coastguard Workeryou should check that your patch does not introduce new warnings. To see scan-build report, follow
227*0d6140beSAndroid Build Coastguard WorkerJenkins link -> Build artifacts -> scan build link for the given run.
228*0d6140beSAndroid Build Coastguard Worker
229*0d6140beSAndroid Build Coastguard WorkerAdding reviewers to the patch
230*0d6140beSAndroid Build Coastguard Worker-----------------------------
231*0d6140beSAndroid Build Coastguard Worker
232*0d6140beSAndroid Build Coastguard WorkerAfter pushing the patch, ideally try to make sure there are some reviewers added to your patch.
233*0d6140beSAndroid Build Coastguard Worker
234*0d6140beSAndroid Build Coastguard Workerflashrom has MAINTAINERS file with people registered for some areas of the code. People who
235*0d6140beSAndroid Build Coastguard Workerare in MAINTAINERS file will be automatically added as reviewers if the patch touches that
236*0d6140beSAndroid Build Coastguard Workerarea. However, not all areas are covered in the file, and it is possible that for the patch you
237*0d6140beSAndroid Build Coastguard Workersent no one is added automatically.
238*0d6140beSAndroid Build Coastguard Worker
239*0d6140beSAndroid Build Coastguard WorkerIf you know someone in the dev community who can help with patch review, add the person(s) you know.
240*0d6140beSAndroid Build Coastguard Worker
241*0d6140beSAndroid Build Coastguard WorkerIn general, it's a good idea to add someone who has a knowledge of whatever the patch is doing,
242*0d6140beSAndroid Build Coastguard Workereven if the person has not been added automatically.
243*0d6140beSAndroid Build Coastguard Worker
244*0d6140beSAndroid Build Coastguard WorkerIf you are new, and don't know anyone, and no one has been added automatically: you can add
245*0d6140beSAndroid Build Coastguard WorkerAnastasia Klimchuk (aklm) as a reviewer.
246*0d6140beSAndroid Build Coastguard Worker
247*0d6140beSAndroid Build Coastguard WorkerGoing through code reviews
248*0d6140beSAndroid Build Coastguard Worker--------------------------
249*0d6140beSAndroid Build Coastguard Worker
250*0d6140beSAndroid Build Coastguard WorkerYou will likely get some comments on your patch, and you will need to fix the comments.
251*0d6140beSAndroid Build Coastguard WorkerAfter doing the work locally, amend your commit ``git commit --amend -s`` and push to Gerrit again.
252*0d6140beSAndroid Build Coastguard WorkerCheck that Change-Id in commit message stays the same. This way Gerrit knows your change belongs
253*0d6140beSAndroid Build Coastguard Workerto the same patch, and will upload new change as new patchset for the same patch.
254*0d6140beSAndroid Build Coastguard Worker
255*0d6140beSAndroid Build Coastguard WorkerAfter uploading the work, go through comments and respond to them. Mark as Done the ones you done
256*0d6140beSAndroid Build Coastguard Workerand mark them as resolved. If there is something that is impossible to do, or maybe you have more questions,
257*0d6140beSAndroid Build Coastguard Workeror maybe you are not sure what you are asked about: respond to a comment **without marking it as resolved**.
258*0d6140beSAndroid Build Coastguard Worker
259*0d6140beSAndroid Build Coastguard WorkerIt is completely fine to ask a clarifying questions if you don't understand what the comment is asking you to do.
260*0d6140beSAndroid Build Coastguard WorkerIf is also fine to explain why a comment can't be done, if you think it can't be done.
261*0d6140beSAndroid Build Coastguard Worker
262*0d6140beSAndroid Build Coastguard WorkerThe patch reviews may take some time, but please don't get discouraged.
263*0d6140beSAndroid Build Coastguard WorkerWe have quite high standards regarding code quality.
264*0d6140beSAndroid Build Coastguard Worker
265*0d6140beSAndroid Build Coastguard WorkerInitial review should include a broad indication of acceptance or rejection of
266*0d6140beSAndroid Build Coastguard Workerthe idea/rationale/motivation or the implementation
267*0d6140beSAndroid Build Coastguard Worker
268*0d6140beSAndroid Build Coastguard WorkerIn general, reviews should focus on the architectural changes and things that
269*0d6140beSAndroid Build Coastguard Workeraffect flashrom as a whole. This includes (but is by no means limited to)
270*0d6140beSAndroid Build Coastguard Workerchanges in APIs and types, safety, portability, extensibility, and
271*0d6140beSAndroid Build Coastguard Workermaintainability. The purpose of reviews is not to create perfect patches, but
272*0d6140beSAndroid Build Coastguard Workerto steer development in the right direction and produce consensus within the
273*0d6140beSAndroid Build Coastguard Workercommunity. The goal of each patch should be to improve the state of the project
274*0d6140beSAndroid Build Coastguard Worker- it does not need to fix all problems of the respective field perfectly.
275*0d6140beSAndroid Build Coastguard Worker
276*0d6140beSAndroid Build Coastguard Worker   New contributors may need more detailed advices and should be told about
277*0d6140beSAndroid Build Coastguard Worker   minor issues like formatting problems more precisely. The result of a review
278*0d6140beSAndroid Build Coastguard Worker   should either be an accepted patch or a guideline how the existing code
279*0d6140beSAndroid Build Coastguard Worker   should be changed to be eventually accepted.
280*0d6140beSAndroid Build Coastguard Worker
281*0d6140beSAndroid Build Coastguard WorkerTo get an idea whether the patch is ready or not, please check :ref:`merge-checklist`.
282*0d6140beSAndroid Build Coastguard Worker
283*0d6140beSAndroid Build Coastguard WorkerIf you sent a patch and later lost interest or no longer have time to follow up on code review,
284*0d6140beSAndroid Build Coastguard Workerplease add a comment saying so. Then, if any of our maintainers are interested in finishing the work,
285*0d6140beSAndroid Build Coastguard Workerthey can take over the patch.
286*0d6140beSAndroid Build Coastguard Worker
287*0d6140beSAndroid Build Coastguard WorkerDownloading patch from Gerrit
288*0d6140beSAndroid Build Coastguard Worker-----------------------------
289*0d6140beSAndroid Build Coastguard Worker
290*0d6140beSAndroid Build Coastguard WorkerSometimes you may need to download a patch into your local repository. This can be needed for example:
291*0d6140beSAndroid Build Coastguard Worker
292*0d6140beSAndroid Build Coastguard Worker* if you want to test someone else's patch,
293*0d6140beSAndroid Build Coastguard Worker* if multiple developers are collaborating on a patch,
294*0d6140beSAndroid Build Coastguard Worker* if you are continuing someone else's work, when original author left or unable to continue.
295*0d6140beSAndroid Build Coastguard Worker
296*0d6140beSAndroid Build Coastguard WorkerFirst prepare local repository: sync to head or to desired tag / commit.
297*0d6140beSAndroid Build Coastguard Worker
298*0d6140beSAndroid Build Coastguard WorkerOpen patch in Gerrit, open "three dot" menu on top-right, open Download patch. Copy Cherry-pick command (pick
299*0d6140beSAndroid Build Coastguard Workerthe relevant tab for you: anonymous http / http / ssh) and run the copied command in your local repo.
300*0d6140beSAndroid Build Coastguard Worker
301*0d6140beSAndroid Build Coastguard WorkerNow you have the commit locally and can do the testing or futher developing. To upload your local changes,
302*0d6140beSAndroid Build Coastguard Workerpush patch to Gerrit again (see :ref:`pushing-a-patch`).
303*0d6140beSAndroid Build Coastguard Worker
304*0d6140beSAndroid Build Coastguard WorkerMake sure people involved in the patch agree that you are pushing new version of someone else's patch,
305*0d6140beSAndroid Build Coastguard Workerso this does not come at a surprise for an original author.
306*0d6140beSAndroid Build Coastguard Worker
307*0d6140beSAndroid Build Coastguard WorkerMerging patches
308*0d6140beSAndroid Build Coastguard Worker---------------
309*0d6140beSAndroid Build Coastguard Worker
310*0d6140beSAndroid Build Coastguard WorkerMerging to branches is limited to the "flashrom developers" group on Gerrit (see also :doc:`/about_flashrom/team`).
311*0d6140beSAndroid Build Coastguard Worker
312*0d6140beSAndroid Build Coastguard WorkerThe list of requirements for the patch to be ready for merging is below, see :ref:`merge-checklist`.
313*0d6140beSAndroid Build Coastguard WorkerSome of the requirements are enforced by Gerrit, but not all of them. In general, a person who clicks
314*0d6140beSAndroid Build Coastguard WorkerSubmit button is responsible to go through Merge checklist. Code reviewers should be aware of the checklist
315*0d6140beSAndroid Build Coastguard Workeras well.
316*0d6140beSAndroid Build Coastguard Worker
317*0d6140beSAndroid Build Coastguard WorkerPatch owners can use the checklist to detect whether the patch is ready for merging or not.
318*0d6140beSAndroid Build Coastguard Worker
319*0d6140beSAndroid Build Coastguard Worker.. _merge-checklist:
320*0d6140beSAndroid Build Coastguard Worker
321*0d6140beSAndroid Build Coastguard WorkerMerge checklist
322*0d6140beSAndroid Build Coastguard Worker^^^^^^^^^^^^^^^
323*0d6140beSAndroid Build Coastguard Worker
324*0d6140beSAndroid Build Coastguard Worker#. Every patch has to be reviewed and needs at least one +2 that was not given by the commit's author.
325*0d6140beSAndroid Build Coastguard Worker   Ideally, people who were actively reviewing the patch and adding comments, would be the ones approving it.
326*0d6140beSAndroid Build Coastguard Worker#. If a patch is authored by more than one person (Co-developed-by), each author may +2 the other author's changes.
327*0d6140beSAndroid Build Coastguard Worker#. Patch needs to get Verified +1 vote, typically from Jenkins build bot. This means the patch builds successfully
328*0d6140beSAndroid Build Coastguard Worker   and all unit tests pass.
329*0d6140beSAndroid Build Coastguard Worker#. Commit message should have Signed-off-by line, see :ref:`sign-off` and align with the rest
330*0d6140beSAndroid Build Coastguard Worker   of the rules for :ref:`commit-message`
331*0d6140beSAndroid Build Coastguard Worker#. All the comments need to be addressed, especially if there was a negative vote in the process of review (-1 or -2).
332*0d6140beSAndroid Build Coastguard Worker#. flashrom developers are people from literally all around the planet, and various timezones. We usually wait
333*0d6140beSAndroid Build Coastguard Worker   for 3 days (3 * 24hours) after the patch is fully approved just in case of last minute concerns from all timezones.
334*0d6140beSAndroid Build Coastguard Worker#. In the case of emergency, merging should not take place within less than 24 hours after the review
335*0d6140beSAndroid Build Coastguard Worker   started (i.e. the first message by a reviewer on Gerrit).
336*0d6140beSAndroid Build Coastguard Worker
337*0d6140beSAndroid Build Coastguard WorkerTo help search for patches which are potential candidates for merging, you can try using this search in Gerrit::
338*0d6140beSAndroid Build Coastguard Worker
339*0d6140beSAndroid Build Coastguard Worker   status:open project:flashrom -is:wip -label:Verified-1 label:Verified+1 -label:Code-Review<0 age:3d is:mergeable is:submittable -has:unresolved
340*0d6140beSAndroid Build Coastguard Worker
341*0d6140beSAndroid Build Coastguard WorkerNote the search is not a replacement for Merge checklist, but it can help find candidates for merging.
342*0d6140beSAndroid Build Coastguard Worker
343*0d6140beSAndroid Build Coastguard Worker.. _bugtracker:
344*0d6140beSAndroid Build Coastguard Worker
345*0d6140beSAndroid Build Coastguard WorkerBugtracker
346*0d6140beSAndroid Build Coastguard Worker==========
347*0d6140beSAndroid Build Coastguard Worker
348*0d6140beSAndroid Build Coastguard WorkerWe have a bugtracker on `<https://ticket.coreboot.org/projects/flashrom>`_.
349*0d6140beSAndroid Build Coastguard WorkerAnyone can view tickets, but to be able to create/update/assign tickets you need an account.
350*0d6140beSAndroid Build Coastguard Worker
351*0d6140beSAndroid Build Coastguard WorkerMirrors
352*0d6140beSAndroid Build Coastguard Worker========
353*0d6140beSAndroid Build Coastguard Worker
354*0d6140beSAndroid Build Coastguard WorkerThe only official repository is https://review.coreboot.org/flashrom ; GitHub and GitLab are just mirrors.
355*0d6140beSAndroid Build Coastguard Worker**Reviewers do not look at pull requests** on mirrors.
356*0d6140beSAndroid Build Coastguard WorkerEven if pull requests were automatically transferred to Gerrit,
357*0d6140beSAndroid Build Coastguard Workerrequirements such as :ref:`sign-off` still present a problem.
358*0d6140beSAndroid Build Coastguard Worker
359*0d6140beSAndroid Build Coastguard WorkerThe quickest and best way to get your patch reviewed and merged is by sending
360*0d6140beSAndroid Build Coastguard Workerit to review.coreboot.org (see :ref:`working-with-Gerrit`). Conveniently, you can use your GitHub, GitLab or
361*0d6140beSAndroid Build Coastguard WorkerGoogle account as an OAuth2 `login method <https://review.coreboot.org/login>`_.
362