Development Guidelines: Difference between revisions

From flashprog
Jump to navigation Jump to search
Dhendrix@flashrom.org/ (talk)
("Dhendrix@flashrom.org/: →‎Sending a patch: ")
m (Correct numbering in `Push your patch')
 
(11 intermediate revisions by 3 users not shown)
Line 13: Line 13:
continues there.
continues there.


=== ''master'' branch ===
The historical ''staging'' branch was eventually renamed to ''master''.
Releases of flashrom were based on this branch from 1.0 to 1.3. However,
as regressions in both functionality and maintainability became visible
around flashrom 1.3, a "flashrom-stable" ''main'' branch was forked from
flashrom 1.2.


The historical ''staging'' branch was finally renamed to ''master''.
=== Flashprog ''main'' Branch ===
As usual there is no quality promise for the state of the code on the
''master'' branch. Even though we will try to keep the regression
rate as low as possible, the main purpose of the branch is to merge
new commits and make them available to a broader audience for testing.


=== Release branches (e.g. ''1.0.x'') ===
Our ''main'' branch is a direct descendant of flashrom-stable which was
discontinued on ''flashrom.org''. It's where we currently focus all our
development. The goal hasn't changed: Merge new commits and make them
available to a broader audience for testing, whilst trying to keep the
regression rate as low as possible.
 
=== Release Branches (e.g. ''1.0.x'') ===


Branching for a new release can happen at any point in time when a
Branching for a new release can happen at any point in time when a
commit (branch point) on ''master'' seems to be in good shape and was
commit (branch point) on ''main'' seems to be in good shape and was
reasonably tested after previous invasive changes. Between the branch
reasonably tested after previous invasive changes. Between the branch
point and the release, every fix pushed for ''master'' for a problem
point and the release, every fix pushed for ''main'' for a problem
that also persists on the release branch shall be backported. The same
that also persists on the release branch shall be backported. The same
also applies after the release for the latest release branch and,
also applies after the release for the latest release branch and,
Line 47: Line 53:
and will only add backported fixes to the release.
and will only add backported fixes to the release.


Unless the branch point (i.e. last common commit of ''master''
The branch point (i.e. last common commit of ''main'' and a release
and a release branch) is an RC, it should be tagged as
branch) should be tagged as '''''p<major>.<minor>''''' (e.g. ''p1.0''),
'''''p<major>.<minor>''''' (e.g. ''p1.0''), to keep track where
to keep track where we are on the ''main'' branch.
we are on the ''master'' branch.


= Patch submission =
= Patch Submission =


== Coding style ==
== 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
Flashprog generally follows the
[https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst Linux kernel style].


The notable exception is line length limit. Our guidelines are:
The notable exception is line length limit. Our guidelines are:
Line 65: Line 71:
* Tables are the only exception to the hard limit and may be as long as needed for practical purposes.
* Tables are the only exception to the hard limit and may be as long as needed for practical purposes.


== Sending a patch ==
== Sending a Patch ==


'''Before submitting a patch for review, put your [[#Sign-off Procedure|Signed-off-by line]] in the commit message.'''
'''Before submitting a patch for review, put your [[#Sign-off Procedure|Signed-off-by line]] in the commit message.'''


Currently there are three ways to submit patches:
Currently there are two ways to submit patches:


1. Via [https://review.coreboot.org/#/q/project:flashrom gerrit on coreboot.org], i.e. ''git push origin HEAD:refs/for/master'' ([https://coreboot.org/Git see instructions])
1. Via [https://review.sourcearcade.org/q/project:flashprog Gerrit on sourcearcade.org],
i.e. ''git push origin HEAD:refs/for/main''


2. Via our [[Mailinglist|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.
2. Via our [[Contact#Mailing_List|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.


3. Github users: See [https://www.flashrom.org/Development_Guidelines#Using_Github Using Github].
Our guidelines borrow  heavily from the
 
[https://doc.coreboot.org/contributing/gerrit_guidelines.html coreboot development guidelines],
Our guidelines borrow  heavily from [http://www.coreboot.org/Development_Guidelines the coreboot development guidelines], and most of them apply to flashrom as well. The really important part is about the Signed-off-by procedure which is quoted [[#Sign-off Procedure|below]].
and most of them apply to flashprog as well. The really important part
is about the Signed-off-by procedure which is quoted [[#Sign-off Procedure|below]].


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 [[Random notes#Command_set_secrets|Command set secrets]].
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 [[Random notes#Command_set_secrets|Command set secrets]].
Line 83: Line 91:
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.
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 [[Mailinglist|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.
If you introduce new features (not flash chips, but stuff like partial
 
programming, support for new external programmers, voltage handling,
For patches that modify convoluted tables like <tt>struct flashchip flashchips[]</tt> 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.
etc) please '''discuss your plans''' on the
 
[[Contact#Mailing_List|mailing list]] first. That way, we can avoid
== Using Github ==
duplicated work and know about how flashprog internals need to be
adjusted and you avoid frustration if there is some disagreement about
the design.


The official Flashrom mirror on Github is: https://github.com/flashrom/flashrom
If you intend to send patches to the mailing list that modify convoluted tables like <tt>struct flashchip flashchips[]</tt> in flashchips.c, it makes 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.


All changes go thru Gerrit on review.coreboot.org before being merged. As a convenience, pull requests may be used for small patches such as adding support for a flash chip or simple compilation fixes (usually <10 lines), however a maintainer will need to see the patch and push it to the upstream review server before it can be merged (See: [https://www.flashrom.org/Development_Guidelines#Merging_to_branches Merging to branches]).
=== Set up a Gerrit Account on review.sourcearcade.org ===


The quickest and best way to get your patch reviewed and merged is by sending it to review.coreboot.org. You may use your Github/OAuth2 credentials. Here's how to do this:
1. Go to https://review.sourcearcade.org/login and create an account.
 
Currently, locally stored on SourceArcade and GitHub credentials can be
=== Set up your Gerrit account on review.coreboot.org ===
used.
 
1. Go to https://review.coreboot.org/login and sign in using your Github credentials.


2. Edit your settings by clicking on the gear icon in the upper right corner.
2. Edit your settings by clicking on the gear icon in the upper right corner.


3. Set your Gerrit username (this is different from your Github username).
3. Upload an SSH public key, or click the button to generate an HTTPS password.
 
4. Add an e-mail address so that Gerrit can send notifications to you about your patch.
 
5. Upload an SSH public key, or click the button to generate an HTTPS password


=== Push your patch ===
=== Push your patch ===


1. Install Change-Id hook: ''gitdir=$(git rev-parse --git-dir); scp -p -P 29418 <gerrit_username>@review.coreboot.org:hooks/commit-msg ${gitdir}/hooks/''
1. Install Git hooks: ''make gitconfig''


2. Add upstream as a remote:
2. If using SSH, update the push URL:
If using SSH: ''git remote add -f upstream ssh://'''<gerrit_username>'''@review.coreboot.org:29418/flashrom.git''.
''git remote set-url --push origin
If using HTTPS: ''git remote add -f upstream https://review.coreboot.org/flashrom''
ssh://'''<gerrit_username>'''@review.sourcearcade.org:29418/flashprog''


3. Check out a new local branch that tracks upstream/master: ''git checkout -b <branch_name> upstream/master''
3. Push to Gerrit: ''git push origin HEAD:refs/for/master%topic=my_wonderful_patch''.


4. Cherry-pick to new local branch: ''git cherry-pick <your_commit_from_local_branch>''
* If using HTTPS you will be prompted for the username and password you set in the Gerrit UI.


5. Push to gerrit: ''git push upstream HEAD:refs/for/master%topic=my_wonderful_patch''. 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.
 
If successful, the Gerrit URL for your patch will be shown in the output.


== Commit message ==
== Commit message ==
Line 139: Line 141:
  Finally, follow the [[#Sign-off Procedure]] to add your sign-off!
  Finally, follow the [[#Sign-off Procedure]] to add your sign-off!
   
   
  <span style="color:red">Signed-off-by: Your Name <your@domain></span>
  <span style="color:#cb4b16">Signed-off-by: Your Name <your@domain></span>




=== Sign-off Procedure ===
=== Sign-off Procedure ===


We employ a similar sign-off procedure [http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html as the Linux kernel developers] do.
We employ a similar sign-off procedure
Add a note such as
[http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html as the Linux kernel developers] do.
Each gerrit commit requires a sign-off line saying that the contributed
code abides by the Developer’s certificate of origin, below.
  Signed-off-by: Random J Developer <random@developer.example.org>
  Signed-off-by: Random J Developer <random@developer.example.org>
to your email/patch if you agree with the Developer's Certificate of Origin 1.1 printed below. Read [https://lkml.org/lkml/2004/5/23/10 this post on the LKML] for rationale (spoiler: SCO).


<span style="color:red">You must use your real name in the Signed-off-by line and in any copyright notices you add.</span> Patches without an associated real name lack provenance and cannot be committed!
Using ''-s'' with ''git commit'' will automatically add a Signed-off-by line
to your commit message. Patches without a Signed-off-by should not be
pushed to Gerrit.
 
You must use a known identity in the Signed-off-by line. Anonymous
contributions cannot be committed! This can be anything sufficient to
identify and contact the source of a contribution, such as your name or
an established alias/nickname. Refer to this
[https://lkml.org/lkml/2004/5/23/10 LKML thread] and the
[https://en.wikipedia.org/wiki/SCO%E2%80%93Linux_disputes SCO-Linux disputes]
for the rationale behind the DCO.


'''Developer's Certificate of Origin 1.1:'''
'''Developer's Certificate of Origin 1.1:'''
Line 173: Line 186:
= Reviews =
= Reviews =


All patches finally have to go through Gerrit. Though, if the author prefers, the actual reviewing process can also take place on the mailing list or on github.
All patches finally have to go through Gerrit. Though, if the author
prefers, the actual reviewing process can also take place on the mailing
list.


All contributions should receive at least a preliminary review within one week of submission by some flashrom developer (if that doesn't happen in time, please be patient).
All contributions should receive at least a preliminary review within
At minimum this should include a broad indication of acceptance or rejection of...
one week of submission by some flashprog developer (if that doesn't
happen in time, please be patient). At minimum this should include a
broad indication of acceptance or rejection of...
* the idea/rationale/motivation,
* the idea/rationale/motivation,
* the implementation
* the implementation
respectively.
respectively.


In general, reviews should focus on the architectural changes and things that affect flashrom as a whole.
In general, reviews should focus on the architectural changes and things
that affect flashprog as a whole.
This includes (but is by no means limited to) changes in APIs and types, safety, portability, extensibility, and maintainability.
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 purpose of reviews is not to create perfect patches, but to steer development in the right direction and produce consensus within the community.
Line 187: Line 205:
NB: New contributors may need more detailed advices and should be told about minor issues like formatting problems more precisely.
NB: 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.
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.
See also [Adding and Reviewing new flash chips].


== Adding/reviewing a new flash chip ==
= Merging to Branches =
# Get the datasheet of the exact type of chip.
# Open <tt>flashchips.c</tt> and <tt>flashchips.h</tt>.
# First, find the best* IDs in the datasheet (*FIXME: this needs to be explained together with the probing somewhere else in detail) and check if the ID exists in <tt>flashchips.h</tt> already
#* If it does but is named after a different chip,
#*:then add a comment regarding the twin and continue by comparing the definition in <tt>flashchips.c</tt> with the datasheet of the twin/new chip as if you would add it but leave out the next step (see below). First you should change the <tt>.name</tt> to reflect the additional chip model (see other chips of naming examples). If you find significant* differences in the chips behavior you have found a so called evil twin (*judging the significance of a difference is quite hard and requires some understanding of flashrom behavior, examples of significant differences are: different sizes of blocks or different opcodes for operations). In that case copy the entry and continue to change that (don't forget to undo the previous changes before).
#* If it does and the name matches too,
#*:the chip is either already added or only the ID was added and you should use that define.
#* If it does not,
#*:then you should add it conforming to the standards/comments in the file.
#: Usually the chip IDs follow a simple scheme: They are all uppercase; first the manufacturer name (like for the manufacturer IDs on top of each paragraph in flashchips.h) followed by an underscore and then the chipname. The latter should in general equal the <tt>.name</tt>, with dots (and other disallowed characters) replaced by underscores. Shared chip IDs typically use the macro name that happened to be added first to flashrom (which is also probably the first one manufactured) and which usually matches the other chips of that series in flashchips.h.
# If possible copy an existing, similar entry in the giant array in <tt>flashchips.c</tt> or start a new one at the right position (according to the comment on top of the array)
# Add <tt>.vendor</tt>, <tt>.name</tt>, IDs selected as explained above and <tt>.total_size</tt>.
# <tt>.page_size</tt> is really hard. Please read this [http://www.flashrom.org/pipermail/flashrom/2013-April/010817.html long explanation], or ignore it for now and set it to <tt>256</tt>.
# We encode various features of flash chips in a bitmask named <tt>.feature_bits</tt>. The various possibilities can be found in <tt>flash.h</tt>.
# <tt>.tested</tt> is used to indicate if the code was tested to work with real hardware, its possible values are defined in <tt>flash.h</tt>. Without any tests it should be set to <tt>TEST_UNTESTED</tt>.
# <tt>.probe</tt> indicates which function is called to fetch IDs from the chip and to compare them with the ones in <tt>.manufacture_id</tt> and <tt>.model_id</tt>. This requires some knowledge or source reading. For most SPI flash chips <tt>probe_spi_rdid</tt> is the right one if the datasheets mentions <tt>0x9f</tt> as an identification/probing opcode.
# <tt>.probe_timing</tt> is only used for non-SPI chips. It indicates the delay after "enter/exit ID mode" commands in microseconds (see <tt>flash.h</tt> for special values).
# <tt>.block_erasers</tt> stores an array of pairs of erase functions (<tt>.block_erase</tt>) with their respective layout (<tt>.eraseblocks</tt>).
## <tt>.block_erase</tt> is similar to the probing function. You should at least check that the opcode named in the function name is matching the respective opcode in the datasheet.
## Two forms of <tt>.eraseblocks</tt> can be distinguished: symmetric and asymmetric layouts. Symmetric means that all blocks that can be erased by an opcode are sized equal. In that case a single range can define the whole layout (e.g. <tt>{4 * 1024, 256}</tt> means 256 blocks of 4 kB each). Asymmetric layouts on the other hand contain differently sized blocks, ordered by their base addresses (e.g. <tt>{{8 * 1024, 1}, {4 * 1024, 2}, {16 * 1024, 7}}</tt> describes a layout that starts with a single 8 kB block, followed by two 4 kB blocks and 7 16 kB blocks at the end).
# <tt>.printlock</tt> is a [http://www.flashrom.org/pipermail/flashrom/2011-May/006495.html misnomer to some extent]. It is misused not only to print (write) protected address ranges of the chip, but also to pretty print the values of the status register(s) - especially true for SPI chips. There are a lot of existing functions for that already and you should reuse one if possible. Comparing the description of the status register in the datasheet of an already supported chip with that of your chip can help to determine if you can reuse a printlock function.
# <tt>.unlock</tt> is called before flashrom wants to modify the chip's contents to disable possible write protections. It is tightly related to the <tt>.printlock</tt> function as it tries to change some of the bits displayed by <tt>.printlock</tt>.
# <tt>.write</tt> and <tt>.read</tt> are function pointers with the obvious meaning. Currently flashrom does only support a single function each. The one that is best supported by existing programmers should be used for now, but others should be noted in a comment if available.
# <tt>.voltage</tt> defines the upper and lower bounds of the supply voltage of the chip. If there are multiple chip models with different allowed voltage ranges, the [http://en.wikipedia.org/wiki/Intersection_(set_theory) intersection] should be used and an appropriate comment added.
# The write [http://www.flashrom.org/pipermail/flashrom/2013-April/010817.html granularity] can be expressed by the <tt>.gran</tt> field. If you think you need something else than the default (<tt>write_gran_256bytes</tt>) then you should definitely ask one of the regular flashrom hackers first. Possible values can be found in <tt>flash.h</tt>.
 
= Merging to branches =


Merging to branches is limited to the "flashrom developers" group on
Merging to branches is limited to the flashprog maintainers. The
Gerrit. This means every patch reviewed somewhere else (e.g. mailing
following rules apply, some are already enforced by Gerrit:
list or github) must finally be pushed to 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.
* Every commit has to be reviewed and needs at least one +2.
* Except, if a commit is authored by more than one person, each author may +2 the other author's changes.
* A maintainer is allowed to raise a +1 to a +2 if they trust the reviewer. If not, they should have a closer look before giving a +2.
* Merging should not take place within less than 24 hours after the review started (i.e. the first message by a reviewer on Gerrit).
* Before merging, a maintainer should have a final look that the patch moves things into the right direction.
* Merging should not take place within less than 24 hours after the patch was sent to review.
* Finally, before hitting ''Submit'', one is reponsible to check that all comments have been addressed, especially if there was a negative review (-1).
* Finally, before hitting ''Submit'', one is reponsible to check that all comments have been addressed, especially if there was a negative review (-1).

Latest revision as of 09:03, 3 May 2024

Branches

Historical

Till the release of flashrom 0.9.9 there was basically a single branch (trunk) where linear development happened. After 0.9.9 it was decided to switch to Git and a two branch model, a stable and a staging branch. This led to some confusion and as nobody who had access to the stable branch had the time to work on it, development continued about one year after the 0.9.9 release on a staging branch at coreboot.org. Despite its name, we strived to keep flashrom's quality and hoped that everything would be merged to stable once work continues there.

The historical staging branch was eventually renamed to master. Releases of flashrom were based on this branch from 1.0 to 1.3. However, as regressions in both functionality and maintainability became visible around flashrom 1.3, a "flashrom-stable" main branch was forked from flashrom 1.2.

Flashprog main Branch

Our main branch is a direct descendant of flashrom-stable which was discontinued on flashrom.org. It's where we currently focus all our development. The goal hasn't changed: Merge new commits and make them available to a broader audience for testing, whilst trying to keep the regression rate as low as possible.

Release Branches (e.g. 1.0.x)

Branching for a new release can happen at any point in time when a commit (branch point) on main seems to be in good shape and was reasonably tested after previous invasive changes. Between the branch point and the release, every fix pushed for main for a problem that also persists on the release branch shall be backported. The same also applies after the release for the latest release branch and, optionally, for any earlier release branch that is still maintained for other reasons (e.g. part of a long term distribution).

Whenever a release branch has no further unmerged commits in queue and is not awaiting backported fixes, a release candidate (RC) can be tagged on that branch. This can also be the original branch point. The RC shall undergo extensive build tests and be publicly advertised as ready for testing. Not less than three days after the last RC that included code changes, a release can be tagged if no regressions showed up.

Release-branch names follow the pattern <major>.<minor>.x (e.g. 1.0.x). The first release of a branch is tagged v<major>.<minor>, without a point-release number (e.g. v1.0). Every following release from the same branch will have a point-release number starting with .1 (e.g. v1.0.1) and will only add backported fixes to the release.

The branch point (i.e. last common commit of main and a release branch) should be tagged as p<major>.<minor> (e.g. p1.0), to keep track where we are on the main branch.

Patch Submission

Coding Style

Flashprog generally follows the Linux kernel style.

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.

Sending a Patch

Before submitting a patch for review, put your Signed-off-by line in the commit message.

Currently there are two ways to submit patches:

1. Via Gerrit on sourcearcade.org, i.e. git push origin HEAD:refs/for/main

2. Via our 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.

Our guidelines borrow heavily from the coreboot development guidelines, and most of them apply to flashprog as well. The really important part is about the Signed-off-by procedure which is quoted below.

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.

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 first. That way, we can avoid duplicated work and know about how flashprog internals need to be adjusted and you avoid frustration if there is some disagreement about the design.

If you intend to send patches to the mailing list that modify convoluted tables like struct flashchip flashchips[] in flashchips.c, it makes 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.

Set up a Gerrit Account on review.sourcearcade.org

1. Go to https://review.sourcearcade.org/login and create an account. Currently, locally stored on SourceArcade and GitHub credentials can be used.

2. Edit your settings by clicking on the gear icon in the upper right corner.

3. Upload an SSH public key, or click the button to generate an HTTPS password.

Push your patch

1. Install Git hooks: make gitconfig

2. If using SSH, update the push URL: git remote set-url --push origin ssh://<gerrit_username>@review.sourcearcade.org:29418/flashprog

3. Push to Gerrit: git push origin HEAD:refs/for/master%topic=my_wonderful_patch.

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

Commit message

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.

Finally, follow the #Sign-off Procedure to add your sign-off!

Signed-off-by: Your Name <your@domain>


Sign-off Procedure

We employ a similar sign-off procedure as the Linux kernel developers do. Each gerrit commit requires a sign-off line saying that the contributed code abides by the Developer’s certificate of origin, below. 

Signed-off-by: Random J Developer <random@developer.example.org>

Using -s with git commit will automatically add a Signed-off-by line to your commit message. Patches without a Signed-off-by should not be pushed to Gerrit.

You must use a known identity in the Signed-off-by line. Anonymous contributions cannot be committed! This can be anything sufficient to identify and contact the source of a contribution, such as your name or an established alias/nickname. Refer to this LKML thread and the SCO-Linux disputes for the rationale behind the DCO.

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 is licensed under the terms of the Creative Commons Attribution-ShareAlike 2.5 License.

Reviews

All patches finally have to go through Gerrit. Though, if the author prefers, the actual reviewing process can also take place on the mailing list.

All contributions should receive at least a preliminary review within one week of submission by some flashprog developer (if that doesn't happen in time, please be patient). At minimum this should include a broad indication of acceptance or rejection of...

  • the idea/rationale/motivation,
  • the implementation

respectively.

In general, reviews should focus on the architectural changes and things that affect flashprog 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. NB: 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. See also [Adding and Reviewing new flash chips].

Merging to Branches

Merging to branches is limited to the flashprog maintainers. The following rules apply, some are already enforced by Gerrit:

  • Every commit has to be reviewed and needs at least one +2.
  • A maintainer is allowed to raise a +1 to a +2 if they trust the reviewer. If not, they should have a closer look before giving a +2.
  • Before merging, a maintainer should have a final look that the patch moves things into the right direction.
  • Merging should not take place within less than 24 hours after the patch was sent to review.
  • Finally, before hitting Submit, one is reponsible to check that all comments have been addressed, especially if there was a negative review (-1).
Retrieved from "https://flashprog.org/wiki/index.php?title=Development_Guidelines&oldid=323"