[flashrom] New development guidelines after 0.9.7

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Thu Jul 18 03:21:47 CEST 2013

Our current development model with elaborate reviews has not been
working for a while now. Therefore, beginning with the release of
flashrom 0.9.7 our strategy on merging patches will change radically.

The current rules regarding committing to subversion can be found here:
Essentially they prohibit merging non-trivial patches without explicit
consent from another community member and is not too specific on the
requirements for an ack.

Below is my proposal for the new rules which works with implicit
consent after timeouts. The exact time values are completely open for
discussion of course.

I would also like to introduce some kind of stable branch where we can
commit important backported patches, if need be, without releasing all
non-critical fixes committed in the meantime (as was the case with for example). I would not have a problem doing this with git,
but I am not so sure if I want to try that with svn... let's pretend
there is a community consensus to continue with svn for now.
Is that sensible to do? AFAIK this would produce odd revision numbers
(which is one of the main arguments pro svn).

I would also like to clean up the commit permissions and name the ones
with commit rights in the wiki.
IMHO the following ppl should have commit rights:
carldani, stefanct, dhendrix, kmalkki, idwer
I am not sure about uwe and twice11, because they are (sadly) not even
active on IRC or the ml. Everyone else's permission should be revoked.

= Reviews =

Contributors without commit rights should receive at least a preliminary review within one week of submission by some committer.
At minimum this should include a broad indication of acceptance or rejection of...
* the idea/rationale/motivation,
* 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.
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 a 

== Acked-by ==

* Trivial stuff like...
** compile fixes (that are tested on all or at least some affected and some ''unaffected'' platforms),
** marking boards/chips/programmers as tested, and
** typo and whitespace fixes etc.
:do not need an Acked-by line from someone else than the author.
* If the patch is something like a board enable, new chip(set) support etc. (i.e. no refactoring, no architectural impact), you can get an ack from someone who is not an experienced developer after he/she has tested the patch, or ack yourself if proof exists that it worked.
* However, for more complicated patches you should get a review from someone who knows the code well if possible.
** If no one replies to the patch submission on the mailing list within 2 weeks, you may assume consent by the community and ack yourself.
** If, however, one of the known community members comments on your submission and requests changes or further information/a rationale etc., you need to come to some consensus with that member about the points complained about (only).
*** If your answer (e.g. in form of a new iteration of the patch including the requested changes or a mail containing the requested information) does not get any replies for three weeks, you may ack yourself.
*** When consensus in all points have been reached, but no explicit ack has been sent, then the submission falls under the same timeout rule again; i.e. if no one replies to your last e-mail within 2 weeks, you may ack yourself.
* Contributions from people without commit rights should not be treated much differently.
: Maintainers should merge them if there is no objection from the community within the timeframes and according to the rule set described above.

= Committing =

Those with commit rights to the (currently subversion-based) source repository should follow the following rules.

* Reviews
** Changes should generally be discussed and reviewed before committing, see [[Development_Guidelines#Reviews|above]]
* The commit log
** Should start with a short sentence summing up the changes of the patch.
** Optionally this should be prefixed with a topic or file name if the changes are targeting one area only.
** End the sentence with a full stop followed by an empty line.
** The body of the commit log should be short, but descriptive: If anyone involved in flashrom reads your comment in a year, she/he shall still be able to understand what your commit is about, without analyzing the code (what was the motivation for the change? what are the differences to the previous behavior?).
** At the end it has to list Signed-off-by lines of all authors and at least one Acked-by line.
** Optionally one can add Tested-by lines to give credit to testers.
* Committing
** If you ack something and the sender has commit rights, let him commit unless he asks you to commit.
*: If the sender does not have commit rights, wait at least 24 hours after sending the ack to give him/her an opportunity to add final refinements or comments.
** After committing please reply to the mailing thread that contains the patch with at least the revision in the body to notify anyone involved of the commit and document it for references in the future and in patchwork or similar facilities.
* Don't share your login(s)/password(s) (should be obvious, but sadly it is not for everyone :)

Kind regards/Mit freundlichen Grüßen, Stefan Tauner

More information about the flashrom mailing list