- flashrom-gerrit - mail.coreboot.org

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Anastasia Klimchuk (Code Review) 18 Jun '23

18 Jun '23

Attention is currently required from: Alexander Goncharov, Angel Pons, Edward O'Callaghan, Peter Marheine, Stefan Reinauer, Swift Geek (Sebastian Grzywna), Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email ) Change subject: doc: Add new Development guidelines ...................................................................... Patch Set 2: (3 comments) File doc/dev_guide/development_guidelines.rst: https://review.coreboot.org/c/flashrom/+/75906/comment/cbd1f4a9_1127ce07 : PS2, Line 11: * This is not displayed as a nested list (and same below). Need to be fixed. https://review.coreboot.org/c/flashrom/+/75906/comment/2bd63f9b_d82cd9ae : PS2, Line 119: * Bullet point lists have different indentation in the doc, and rendered with different indentation in html. Fix so that indentation is consistent (pick one and use it for all bullet point lists). https://review.coreboot.org/c/flashrom/+/75906/comment/4bba9294_dd5ec3d1 : PS2, Line 194: you can add Anastasia Klimchuk as a reviewer Is anyone interested to put your name here too? ;) -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sun, 18 Jun 2023 07:39:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Anastasia Klimchuk (Code Review) 18 Jun '23

18 Jun '23

Attention is currently required from: Alexander Goncharov, Angel Pons, Edward O'Callaghan, Peter Marheine, Stefan Reinauer, Swift Geek (Sebastian Grzywna), Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email ) Change subject: doc: Add new Development guidelines ...................................................................... Patch Set 2: (3 comments) Patchset: PS1: > As a reference we have done a brainstorming session https://docs.google. […] Also I want to mention that some sections that used to be a part of Dev guidelines will be moved into other pages: How to add a chip Release process History One thing that potentially could be added, is one paragraph about branches, just general info. Not in the beginning, but at the end of guidelines. What do people think, do we need some info about branches in this doc? File doc/dev_guide/development_guidelines.rst: https://review.coreboot.org/c/flashrom/+/75906/comment/b695bd67_79eab45d : PS1, Line 2: Development Guidelilnes > typo Done, thanks for catching https://review.coreboot.org/c/flashrom/+/75906/comment/04878dcc_a11e38ea : PS1, Line 5: Intro, is there anything useful to say? > Is that a leftover from some draft? Yes and no: I was thinking whether the intro is needed, and and if yes what should be there. I didn't have a clear idea what to say in the intro, so decided to push the patch like this in case other people have ideas. If no one thinks it's needed, I will just delete this line. -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sun, 18 Jun 2023 07:21:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Anastasia Klimchuk (Code Review) 18 Jun '23

18 Jun '23

Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Angel Pons, Edward O'Callaghan, Peter Marheine, Stefan Reinauer, Thomas Heijligen. Hello Alexander Goncharov, Angel Pons, Edward O'Callaghan, Peter Marheine, Stefan Reinauer, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/75906?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+2 by Stefan Reinauer, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: doc: Add new Development guidelines ...................................................................... doc: Add new Development guidelines Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/contact.rst A doc/dev_guide/development_guidelines.rst M doc/dev_guide/index.rst 3 files changed, 296 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/75906/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Swift Geek (Sebastian Grzywna) (Code Review) 17 Jun '23

17 Jun '23

Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Angel Pons, Edward O'Callaghan, Peter Marheine, Thomas Heijligen. Swift Geek (Sebastian Grzywna) has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email ) Change subject: doc: Add new Development guidelines ...................................................................... Patch Set 1: (1 comment) File doc/dev_guide/development_guidelines.rst: https://review.coreboot.org/c/flashrom/+/75906/comment/b3246360_1847e7fd : PS1, Line 5: Intro, is there anything useful to say? Is that a leftover from some draft? -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 17 Jun 2023 19:54:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Swift Geek (Sebastian Grzywna) (Code Review) 17 Jun '23

17 Jun '23

Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Angel Pons, Edward O'Callaghan, Peter Marheine, Thomas Heijligen. Swift Geek (Sebastian Grzywna) has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email ) Change subject: doc: Add new Development guidelines ...................................................................... Patch Set 1: (1 comment) File doc/dev_guide/development_guidelines.rst: https://review.coreboot.org/c/flashrom/+/75906/comment/740cbcde_32e91f68 : PS1, Line 2: Development Guidelilnes typo -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 17 Jun 2023 19:47:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: Switch over master branch to main
by Anastasia Klimchuk (Code Review) 17 Jun '23

17 Jun '23

Attention is currently required from: Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75706?usp=email ) Change subject: Switch over master branch to main ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > More details about Dev guidelines: plain conversion to rst is already done in CB:74923 but what I ne […] Here are new Dev guidelines! CB:75906 :) -- To view, visit https://review.coreboot.org/c/flashrom/+/75706?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8f4a377735f3f6ab4a22006949ff294a218bdf22 Gerrit-Change-Number: 75706 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sat, 17 Jun 2023 14:42:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Stefan Reinauer (Code Review) 17 Jun '23

17 Jun '23

Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Angel Pons, Edward O'Callaghan, Peter Marheine, Thomas Heijligen. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email ) Change subject: doc: Add new Development guidelines ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 17 Jun 2023 14:36:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Anastasia Klimchuk (Code Review) 17 Jun '23

17 Jun '23

Attention is currently required from: Alexander Goncharov, Angel Pons, Edward O'Callaghan, Peter Marheine, Stefan Reinauer, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email ) Change subject: doc: Add new Development guidelines ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: As a reference we have done a brainstorming session https://docs.google.com/document/d/1GHwUiKAUlqXifzXh7hlMpfGe56cyFc8Udn8zsUt… so the patch is taking that session and existing guidelines into account. Comments welcome! -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 17 Jun 2023 14:21:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add new Development guidelines
by Anastasia Klimchuk (Code Review) 17 Jun '23

17 Jun '23

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email ) Change subject: doc: Add new Development guidelines ...................................................................... doc: Add new Development guidelines Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/contact.rst A doc/dev_guide/development_guidelines.rst M doc/dev_guide/index.rst 3 files changed, 296 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/75906/1 diff --git a/doc/contact.rst b/doc/contact.rst index dc89063..220d73f 100644 --- a/doc/contact.rst +++ b/doc/contact.rst @@ -3,6 +3,8 @@ .. The extra = is needed to prevent git from throwing a `leftover conflict marker` error when commiting. +.. _mailing list: + Mailing List ------------ Flashrom related mails are welcome on the flashrom mailing list at `flashrom(a)flashrom.org <mailto:flashrom@flashrom.org>`_. diff --git a/doc/dev_guide/development_guidelines.rst b/doc/dev_guide/development_guidelines.rst new file mode 100644 index 0000000..e9ed165 --- /dev/null +++ b/doc/dev_guide/development_guidelines.rst @@ -0,0 +1,293 @@ +======================= +Development Guidelilnes +======================= + +Intro, is there anything useful to say? + +Set up the git repository and dev environment +============================================= + +#. Clone git repository + * If using https: :code:`git clone "https://review.coreboot.org/flashrom"` + * If using ssh: :code:`git clone "ssh://aklm@review.coreboot.org:29418/flashrom"` +#. Follow the build guidelines to install dependencies :doc:`building_from_source` +#. Install Git hooks: :code:`./util/git-hooks/install.sh` +#. Add upstream as a remote: + * If using https: :code:`git remote add -f upstream https://review.coreboot.org/flashrom` + * If using ssh: :code:`git remote add -f upstream ssh://<gerrit_username>@review.coreboot.org:29418/flashrom` +#. Check out a new local branch that tracks :code:`upstream/master`: :code:`git checkout -b <branch_name> upstream/master` +#. Every patch is required to be signed-off (see also :ref:`sign-off`). + Set up your ``user.name`` and ``user.email`` in git config, and don't forget + to ``-s`` when creating a commit. +#. See also build guidelines :doc:`building_from_source` and `git docs <https://git-scm.com/doc>`_ + +Creating your patch +=================== + +Create a commit and sign-off ``git commit -s``. + +Commit message +-------------- + +Commit messages shall have the following format: + +.. code-block:: + + <component>: Short description (up to 72 characters) + + This is a long description. Max width of each line in the description + is 72 characters. It is separated from the summary by a blank line. You + may skip the long description if the short description is sufficient, + for example "flashchips: Add FOO25Q128" to add FOO25Q128 chip support. + + You may have multiple paragraphs in the long description, but please + do not write a novel here. For non-trivial changes you must explain + what your patch does, why, and how it was tested. + + Finally, follow the sign-off procedure to add your sign-off! + + Signed-off-by: Your Name <your@domain> + +Commit message should include: + +* Commit title +* Commit description: explain what the patch is doing, or what it is fixing. +* Testing information: how did you test the patch. +* Signed-off-by line (see below :ref:`sign-off`) +* If applicable, link to the ticket in the bugtracker `<https://ticket.coreboot.org/projects/flashrom>`_ +* Change-Id for Gerrit. If commit message doesn't have Change-Id, you forgot to install git hooks. + +.. _sign-off: + +Sign-off procedure +^^^^^^^^^^^^^^^^^^ + +We employ a similar sign-off procedure as the `Linux kernel developers +<http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_…>`_ +do. Add a note such as + +.. code-block:: + + Signed-off-by: Random J Developer <random(a)developer.example.org> + +to your email/patch if you agree with the Developer's Certificate of Origin 1.1 +printed below. Read `this post on the LKML +<https://lkml.org/lkml/2004/5/23/10>`_ for rationale (spoiler: SCO). + +You must use your known identity in the ``Signed-off-by`` line and in any +copyright notices you add. Anonymous patches lack provenance and cannot be +committed! + +Developer's Certificate of Origin 1.1 +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + By making a contribution to this project, I certify that: + + (a) The contribution was created in whole or in part by me and I have + the right to submit it under the open source license indicated in the file; or + + (b) The contribution is based upon previous work that, to the best of my + knowledge, is covered under an appropriate open source license and I have the + right under that license to submit that work with modifications, whether created + in whole or in part by me, under the same open source license (unless I am + permitted to submit under a different license), as indicated in the file; or + + (c) The contribution was provided directly to me by some other person who + certified (a), (b) or (c) and I have not modified it; and + + (d) In the case of each of (a), (b), or (c), I understand and agree that + this project and the contribution are public and that a record of the contribution + (including all personal information I submit with it, including my sign-off) is + maintained indefinitely and may be redistributed consistent with this project or the + open source license indicated in the file. + +.. note:: + + The `Developer's Certificate of Origin 1.1 + <http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_…>`_ + is licensed under the terms of the `Creative Commons Attribution-ShareAlike + 2.5 License <http://creativecommons.org/licenses/by-sa/2.5/>`_. + +Coding style +------------ + +Flashrom generally follows Linux kernel style: +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst + +The notable exception is line length limit. Our guidelines are: + + * 80-columns soft limit for most code and comments. This is to encourage simple design and concise naming. + * 112-columns hard limit. Use this to reduce line breaks in cases where they + harm grep-ability or overall readability, such as print statements and + function signatures. Don't abuse this for long variable/function names or + deep nesting. + * Tables are the only exception to the hard limit and may be as long as needed + for practical purposes. + + +Testing a patch +--------------- + +We expect the patch to be approproately tested by the patch owner. +Please add the testing information in commit message, for example that could be some of these: +programmer you were using, programmer params, chip, OS, operations you were running +(read/write/erase/verify), and anything else that is relevant. + +Working with Gerrit +=================== + +All of the patches and code reviews need to go via +`Gerrit on review.coreboot.org <https://review.coreboot.org/#/q/project:flashrom>`_. +While there it is technically possible to send a patch to the mailing list, that patch +still needs to be pushed to Gerrit by someone. We treat patches on the mailing list as a very +exceptional situation. Normal process is to push a patch to Gerrit. +Please read below for instructions and check `official Gerrit documentation <https://gerrit-review.googlesource.com/Documentation/>`_. + +Creating an account +--------------------- + +#. Go to https://review.coreboot.org/login and sign in using the credentials of + your choice. +#. Edit your settings by clicking on the gear icon in the upper right corner. +#. Set your Gerrit username (this may be the different from the username of an + external account you log in with). +#. Add an e-mail address so that Gerrit can send notifications to you about + your patch. +#. Upload an SSH public key, or click the button to generate an HTTPS password. + +Pushing a patch +--------------- + +To push patch to Gerrit, use the follow command: :code:`git push upstream HEAD:refs/for/master`. + + * If using HTTPS you will be prompted for the username and password you + set in the Gerrit UI. + * If successful, the Gerrit URL for your patch will be shown in the output. + +There is an option to add a topic to the patch. For one-off standalone patches this +is not necessary. However if your patch is a part of a larger effort, especially if the +work involves multiple contributors, it can be useful to mark that the patch belongs +to a certain topic. + +Adding a topic makes it easy to search "all the patches by the topic", even if the patches +have been authored by multiple people. + +To add a topic, push with the command: `git push upstream HEAD:refs/for/master%topic=example_topic`. +Alternatively, you can add a topic from a Gerrit UI after the patch in pushed +(on the top-left section) of patch UI. + +Adding reviewers to the patch +----------------------------- + +After pushing the patch, ideally try to make sure there are some reviewers added to your patch. + +flashrom has MAINTAINERS file with people registered for some areas of the code. People who +are in MAINTAINERS file will be automatically added as reviewers if the patch touches that +area. However, not all areas are covered in the file, and it is possible that for the patch you +sent no one is added automatically. + +If you know someone in the dev community who can help with patch review, add the person(s) you know. + +In general, it's a good idea to add someone who has a knowledge of whatever the patch is doing, +even if the person has not been added automatically. + +If you are new, and don't know anyone, and no one has been added automatically: you can add Anastasia Klimchuk as a reviewer. + +Going through code reviews +-------------------------- + +You will likely get some comments on your patch, and you will need to fix the comments. +After doing the work locally, amend you commit ``git commit --amend -s`` and push to Gerrit again. +Check that Change-Id in commit message stays the same. This way Gerrit knows your change belongs +to the same patch, and will upload new change as new patchset for the same patch. + +After uploading the work, go through comments and respond to them. Mark as Done the ones you done +and mark them as resolved. If there is something that is impossible to do, or maybe you have more questions, +or maybe you are not sure what you are asked about: respond to a comment **without marking it as resolved**. + +It is completely fine to ask a clarifying questions if you don't understand what the comment is asking you to do. +If is also fine to explain why a comment can't be done, if you think it can't be done. + +The patch is ready when all comments are resolved and the patch is approved (has +2 vote). + +Mirrors +======== + +The only official repository is https://review.coreboot.org/flashrom and GitHub and GitLab are just mirrors. +**Reviewers do not look at pull requests.** +Even if pull requests were automatically transferred to Gerrit, +requirements such as :ref:`sign-off` still present a problem. + +The quickest and best way to get your patch reviewed and merged is by sending +it to review.coreboot.org. Conveniently, you can use your GitHub, GitLab or +Google account as an OAuth2 `login method <https://review.coreboot.org/login>`_. + +Code review process +=================== + +Our guidelines borrow heavily from `coreboot coding style +<https://doc.coreboot.org/contributing/coding_style.html>`_ and `Gerrit +guidelines <https://doc.coreboot.org/contributing/gerrit_guidelines.html>`_, +and most of them apply to flashrom as well. The really important part is about +the :ref:`sign-off procedure <sign-off>`. + +We try to **reuse as much code as possible** and create new files only if +absolutely needed, so if you find a function somewhere in the tree which +already does what you want, please use it. + +The **patch reviews** may take some time, but please don't get discouraged. +We have quite high standards regarding code quality. + +If you introduce new features (not flash chips, but stuff like partial +programming, support for new external programmers, voltage handling, etc) +please **discuss your plans** on the :ref:`mailing list` first. That way, we +can avoid duplicated work and know about how flashrom internals need to be +adjusted and you avoid frustration if there is some disagreement about the +design. + +Initial review should include a broad indication of acceptance or rejection of +the idea/rationale/motivation or the implementation + +In general, reviews should focus on the architectural changes and things that +affect flashrom as a whole. This includes (but is by no means limited to) +changes in APIs and types, safety, portability, extensibility, and +maintainability. The purpose of reviews is not to create perfect patches, but +to steer development in the right direction and produce consensus within the +community. The goal of each patch should be to improve the state of the project +- it does not need to fix all problems of the respective field perfectly. + +.. note:: + + New contributors may need more detailed advices and should be told about + minor issues like formatting problems more precisely. The result of a review + should either be an accepted patch or a guideline how the existing code + should be changed to be eventually accepted. + +If you sent a patch and later lost interest or no longer have time to follow up on code review, +please add a comment saying so. Then, if any of our maintainers are interested in finishing the work, +they can take over the patch. + +Merging patches +=============== + +Merging to branches is limited to the "flashrom developers" group on Gerrit. +The following rules apply, some are already +enforced by Gerrit: + + * Every commit has to be reviewed and needs at least one +2 that was not given + by the commit's author. + * Except, if a commit is authored by more than one person, each author may +2 + the other author's changes. + * flashrom developers are people from literally all around the planet, and various + timezones, so we usually wait for 3 days after the patch is fully approved just in case + of last minute concerns from all timezones. + * In the case of emergency, merging should not take place within less than 24 hours after the review + started (i.e. the first message by a reviewer on Gerrit). + * Finally, before hitting Submit, one is reponsible to check that all comments + have been addressed, especially if there was a negative review (-1). + +Bugtracker +========== + +We have a bugtracker on `<https://ticket.coreboot.org/projects/flashrom>`_. +Anyone can view tickets, but to be able to create/update/assign tickets you need an account. diff --git a/doc/dev_guide/index.rst b/doc/dev_guide/index.rst index b540127..d6aa30d 100644 --- a/doc/dev_guide/index.rst +++ b/doc/dev_guide/index.rst @@ -6,3 +6,4 @@ building_from_source building_with_make + development_guidelines -- To view, visit https://review.coreboot.org/c/flashrom/+/75906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fe9ab2e27fead8e795138294219b11240f15928 Gerrit-Change-Number: 75906 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[master]: Revert "MAINTAINERS: Add Arthur Heymans for sb600spi"
by Anastasia Klimchuk (Code Review) 17 Jun '23

17 Jun '23

Attention is currently required from: Arthur Heymans. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75901?usp=email ) Change subject: Revert "MAINTAINERS: Add Arthur Heymans for sb600spi" ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/75901?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie538337a409f606b717d7104fad346b708f9e2f8 Gerrit-Change-Number: 75901 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 17 Jun 2023 11:12:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit