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