Hi Anastasia,

thank you for getting this started.

For the mailing list, especially reviews, text-only messages (i.e. no
HTML) are preferred. If one would export the wiki source (not sure if
there is a better way to do it than to copy-paste into a text editor),
and after breaking long lines, it can also be run through `diff` quite
nicely. Not 100% sure if it would have helped a lot here, moving things
around can create rather confusing diffs.

I tried to comment as best as I could inline on what was left of the
HTML.

Nico

On 06.03.22 05:04, Anastasia Klimchuk wrote:
> -----------------------------------------------------------------
> *Patch Submission*
> Coding style (unchanged)
>
> 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 the 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.
>
> GitHub

Why start with GitHub if it's not the preferred way?

>
> The official Flashrom mirror on GitHub is:
> https://github.com/flashrom/flashrom
>
> Importantly, GitHub repo is only a mirror, so all changes must go through
> Gerrit on review.coreboot.org in order to be merged, even small patches
> such as adding support for a flash chip. For this reason, reviewers do not
> look at pull requests. Even if pull requests were automatically transferred
> to Gerrit, requirements such as the #Sign-off Procedure
> <https://www.flashrom.org/Development_Guidelines#Sign-off_Procedure> still
> present a problem.

Change seems ok to me. Can't tell if really nobody is looking at PRs,
though.

>
> 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. Please continue
> reading, the instructions are below.

Please add a link.

> Preparing a patch
>
> Before sending a patch for review, put your Signed-off-by line
> <https://www.flashrom.org/Development_Guidelines#Sign-off_Procedure> in the
> commit message.
>
> Currently there are two ways to send patches for review:
>
>    1.
>
>    Our strong preference, especially for large patches, is via Gerrit on
>    review.coreboot.org <https://review.coreboot.org/#/q/project:flashrom>,
>    i.e. git push origin HEAD:refs/for/master

SGTM

>    2.
>
>    For small patches, with the reviewer’s agreement, there is an option to

How should one get the agreement before sending the patch?

>    send via our mailing list <https://www.flashrom.org/Contact#Mailing_List>.
>    When sending a patch via the mailing list, send it in-line instead of as an
>    attachment so that reviewers can easily comment on specific parts of it.
>    However, eventually the reviewer will need to push patch to gerrit anyway.

*G*errit with upper-case G, please.

>
> Our guidelines borrow heavily from the coreboot development guidelines
> <https://doc.coreboot.org/contributing/index.html>, and most of them apply

That link seems too generic, it also points to coreboot's project ideas
and GSoC page. If there is no good link, maybe it's time to just drop
this paragraph?

> to flashrom as well. The really important part is about the #Sign-off
> Procedure
> <https://www.flashrom.org/Development_Guidelines#Sign-off_Procedure>.
>
> 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 (even if it is for a totally different chip),
> please use it. See also Command set secrets
> <https://www.flashrom.org/Random_notes#Command_set_secrets>.
>
> The patch reviews may sound harsh, but please don't get discouraged. We try
> to merge simple patches after one or two iterations and complicated ones as
> soon as possible, but 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 mailing list
> <https://www.flashrom.org/Contact#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.
>
> For patches that modify convoluted tables like struct flashchip flashchips[]
> in flashchips.c it may make sense to increase the lines of context to
> include enough information directly in the patch for reviewers (for example
> to include the chip names when changing other parameters like .voltage). To
> do this with git use git format-patch -U5 where 5 is an example for the
> number of lines of context you want.

> Commit message (unchanged)
>
> Commit messages shall have the following format:
>
> <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.
>
> TEST=tests that you performed
>
> Finally, follow the #Sign-off Procedure
> <https://www.flashrom.org/Development_Guidelines#Sign-off_Procedure> to add
> your sign-off!
>
> Signed-off-by: Your Name <your(a)domain>
>
>
> Sign-off Procedure (unchanged)
>
> We employ a similar sign-off procedure as the Linux kernel developers
> <http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html>
> do. Add a note such as
>
> 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 real name in the Signed-off-by line and in any copyright
> notices you add. Patches without an associated real name 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_releases/2004/2004_05_24_dco.html>
> is licensed under the terms of the Creative Commons Attribution-ShareAlike
> 2.5 License <http://creativecommons.org/licenses/by-sa/2.5/>.

> Submitting a patch using Gerrit

Sounds like a very good idea to make this guide more generic. I wonder
if it shouldn't be on its own page, though? We should link to it of
course.

Also, something like "Extra steps if you cloned from GitHub" might be
nice so we don't have to drop these things.

> Setting up a Gerrit account

> All changes have
> to go through Gerrit on review.coreboot.org in order to be merged, even
> small patches such as adding support for a flash chip. Our Gerrit supports
> multiple authentication methods <https://review.coreboot.org/login>
> including OAUTH2, e.g. Google, GitHub or GitLab, or also OpenID.
>
>    1.
>
>    Go to https://review.coreboot.org/login and sign in.
>    2.
>
>    Edit your settings by clicking on the gear icon in the upper right
>    corner.
>    3.
>
>    Set your Gerrit username.
>    4.
>
>    Add an email address so that you receive notifications when others
>    commented or reviewed your patch.
>    5.
>
>    Add a SSH public key to your account, or click the button to generate an
>    HTTPS password.
>
> Preparing your local repository
>
> Open https://review.coreboot.org/admin/repos/flashrom and choose your
> desired method to clone the repository. Supported methods are HTTPS and
> SSH. The same method will also be used when you push your changes to Gerrit
> later.
>
> Also, make sure to install the Change-Id hook. This generates a unique ID
> which is appended to your commit message. It is used by Gerrit to identify
> if a patch with the same ID exists, and if so it will add a new version to
> it called “patchset”.

Isn't this taken care of by `make`?

>
> If you are just about getting a fresh copy of the flashrom repository, then
> you can use the command which you can find under “Clone with commit-msg
> hook”.

I never used this, might be new. How well does it work with our
automatism?

> If you have an existing copy of the repository and you need to
> install the hook afterwards, then you can run this command within your
> flashrom directory
>
>
>    -
>
>    mkdir -p .git/hooks && curl -Lo `git rev-parse
>    --git-dir`/hooks/commit-msg
>    https://review.coreboot.org/tools/hooks/commit-msg; chmod +x `git
>    rev-parse --git-dir`/hooks/commit-msg)
>
> Pushing your patch to Gerrit
>
>    1.
>
>    Check out a new local branch that tracks origin/master: git checkout -b
>    <branch_name> origin/master

Seems like something for the "Extra GitHub steps" category. Otherwise
one would just want to checkout `master` right?

>    2.
>
>    Do your changes
>    3.
>
>    Add your changes using `git add` and create a commit using `git commit
>    -s`
>    4.
>
>    Push to gerrit: git push origin HEAD:refs/for/master.

NB. This might be a candidate for a tl;dr for experienced Git users.

>
>
>    -
>
>    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.
>
>