Development Guidelines: Difference between revisions
No edit summary |
(patch context) |
||
Line 8: | Line 8: | ||
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, 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. | ||
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 subversion use '''svn diff --diff-cmd diff -x "-c 5"'''; or with git use '''git diff -U5''' where 5 is an example for the number of context you want. | |||
== Committing == | == Committing == |
Revision as of 03:55, 7 July 2011
Patch submission
The following guidelines are for coreboot, but most of them apply to flashrom as well: coreboot development guidelines. The really important part is about the Signed-off-by 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. Most chips work fine with probe_jedec() even if the command sequence seems to differ at first glance. 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 after a maximum of three iterations.
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 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 subversion use svn diff --diff-cmd diff -x "-c 5"; or with git use git diff -U5 where 5 is an example for the number of context you want.
Committing
Those with commit rights to the (currently subversion-based) source repository should follow the following rules.
- Existence of an Acked-by
- Except for compile fixes and marking boards/chips/programmers as OK, you need an ack from someone else.
- If the patch is needed by someone who is not an experienced developer, you can get an ack from him/her if it is stuff like board enables, chip support, chipset support, ... (no refactoring, no architectural impact) after he/she has tested the patch.
- However, if a patch refactors an interface or changes cosmetics of one of the big tables or changes the command line interface, you should get a review from someone who knows the code well.
- If you ack something and the sender has commit rights, let him commit unless he asks you to commit.
- The commit message should meet the same requirements that coreboot's commit message have.
- 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 :)