On Thursday 31 March 2011 22:20:50 Mark Hatle wrote: > BTW a brief comment from the TSC after they briefly reviewed this draft at > the meeting today. > > Between draft 2 and draft 3, the concept of an "Integrated-by" tag line was > added for unmodified patches and commits. It's been decided that this > isn't a good idea, and we'll use Signed-off-by for all instances. > > It's also suggested to reference something similar to: > > http://www.pokylinux.org/doc/poky-handbook.html > > (Scroll to the bottom, Chapter 6) > > the "Developer's Certificate of Origin" which is given by using the > "Signed-off-by" line. > > Again, the whole point of the Signed-off-by: line and external references > is to document the provenance of the changes, be it an individual recipe's > patch set or the overall OpenEmbedded repository commits. > > --Mark > > On 3/31/11 12:57 PM, Mark Hatle wrote: > > This is the third draft of the guidelines... All previous feedback has > > been incorporated... > > > > It is the first of the drafts being sent to the OpenEmbedded-devel > > mailing list for comments. The intention of the TSC is to use the > > following guidelines to help increase the quality of the commit and > > patches within Open Embedded. Please review the following guidelines and > > let me know if you have any questions, comments or concerns with them. > > > > ---- > > > > This set of guidelines is intended to help both the developer and > > reviewers of changes determine reasonable patch and change commit > > messages. Often when working with the code, you forget that not everyone > > is as familiar with the problem and/or fix as you are. Often the next > > person in the code doesn't understand what or why something is done so > > they quickly look at patch and commit messages. Unless these messages > > are clear it will be difficult to understand the relevance of a given > > change and how future changes may impact previous decisions. > > > > Both patches and commit messages require the same attention and basic > > details, however the purposes of the messages are slightly different. A > > patch needs to describe a specific change that fixes an individual > > problem in a component, while a commit message describes one or more > > changes to the overall project or system. > > > > By following these guidelines we will have a better record of the > > problems and solutions made over the course of development. It will also > > help establish a clear provenance of all of the code and changes within > > the development. > > > > General Information > > ------------------- > > > > While specific to the Linux kernel development, the following could also > > be considered a general guide for any Open Source development: > > > > http://ldn.linuxfoundation.org/book/how-participate-linux-community > > > > Many of the guidelines in this document are related to the items in that > > information. > > > > Pay particular attention to section 5.3 that talks about patch > > preparation. They key thing to remember is to break up your changes into > > logical sections. Otherwise you run the risk of not being able to even > > explain the purpose of a change in the patch headers! > > > > Patch and Commit Headers > > ------------------------ > > There seems to always be a question or two surrounding what a person > > should put in a patch header, or commit message. > > > > The general rules always apply, those being that there is a single line > > short log or summary of the change (think of this as the subject when the > > patch is e-mailed), and then the more detailed long log, and closure > > with tag lines like the "Signed-off-by:" or "Integrated-by:" > > > > See the examples below for examples and details. > > > > New development > > --------------- > > > > A minimal patch or commit message would be of the format: > > Short log / Statement of what needed to be changed. > > > > (Optional pointers to external resources, such as defect tracking) > > > > The intent of your change. > > > > (Optional, if it's not clear from above) how your change resolves the > > issues in the first part. > > > > Tag line(s) at the end. > > > > For example: > > foobar: Adjusted the foo setting in bar > > > > When using foobar on systems with less than a gigabyte of RAM common > > usage patterns often result in an Out-of-memory condition causing > > slowdowns and unexpected application termination. > > > > Low-memory systems should continue to function without running into > > memory-starvation conditions with minimal cost to systems with more > > available memory. High-memory systems will be less able to use the > > full extent of the system, a dynamically tunable option may be best, > > long-term. > > > > The foo setting in bar was decreased from X to X-50% in order to > > ensure we don't exhaust all system memory with foobar threads. > > > > Signed-off-by: Joe Developer <[email protected]> > > > > The minimal commit is good for new code development and simple changes. > > > > The single short log message indicates what needed to be changed. It > > should begin with an indicator as to the primary item changed by this > > patch, followed by a short summary of the change. In the above case > > we're indicating that we've changed the "foobar" item, by "adjusting the > > foo setting in bar". > > > > Optionally, you may include pointers to defects this change corrects. > > Unless the defect format is specified by the component you are > > modifying, it is suggested that you use a full URL to specify the > > reference to the defect information. > > > > You must then have a full description of the change. Specifying the > > intent of your change and if necessary how the change resolves the > > issue. > > > > As mentioned above this is intended to answer the "what were you > > thinking" questions down the road and to know what other impacts are > > likely to result of the change needs to be reverted. It also allows for > > better solutions if someone comes along later and agrees with paragraph > > 1 (the problem statement) and either disagrees with paragraph 2 > > (the design) or paragraph 3 (the implementation). > > > > Finally one or more tag lines should exist. Each developer responsible > > for working on the patch is responsible for adding a Signed-off-by: tag > > line. > > > > It is not acceptable to have an empty or non-existent header, or just a > > single line message. The summary and description is required for all > > changes. > > > > Importing from elsewhere > > ----------------------------- > > If you are importing work from somewhere else, the minimum commit message > > is not enough. It does not clearly establish the provenance of the code. > > The following specified the guidelines required for importing code from > > elsewhere. > > > > By default you should keep the original authors summary and commit > > message, along with any Signed-off-by: information. If the original > > change that was imported does not have a summary and/or commit message > > from the original author, it is still your responsibility to add them to > > the patch. Just as if you wrote the code, you should be able to clearly > > explain what the change does. It is also necessary to document the > > original author of the change. You should indicate the original author > > by simply stating "written by" or "posted to the ... mailing list by". > > Only the original author can commit using a Signed-off-by: tag line. > > > > It is also required that the origin of the work be fully documented. The > > origin should be specified as part of the commit message in a way that > > an average developer would be able to track down the original code. URLs > > should reference original authoritative sources and not mirrors. > > > > If changes were required to the patch, they should be documented as well. > > > > Finally a tag line should be added to indicate that you worked with the > > patch. The "Integrated-by:" tag line should be used to indicate that you > > did not need to modify the patch, it was integrated unchanged. The > > "Signed-off-by:" tag line should only be used when changes to the > > original patch were necessary. > > > > For example, if you were to pull in the patch from the above example unmodified: > > foobar: Adjusted the foo setting in bar > > > > When using foobar on systems with less than a gigabyte of RAM common > > usage patterns often result in an Out-of-memory condition causing > > slowdowns and unexpected application termination. > > > > Low-memory systems should continue to function without running into > > memory-starvation conditions with minimal cost to systems with more > > available memory. High-memory systems will be less able to use the > > full extent of the system, a dynamically tunable option may be best, > > long-term. > > > > The foo setting in bar was decreased from X to X-50% in order to > > ensure we don't exhaust all system memory with foobar threads. > > > > Signed-off-by: Joe Developer <[email protected]> > > > > The foobar recipe was imported from the OpenEmbedded git server > > (git://git.openembedded.org/openembedded) as of commit id > > b65a0e0c84cf489bfa00d6aa6c48abc5a237100f. > > > > Integrated-by: Your Name <[email protected]> > > > > The above example shows a clear way for a developer to find the original > > source of the change. It indicates that the open embedded developer > > simply integrated the change into the system without making any further > > modifications. > > > > However in the same example, if the author did have to make some changes > > to the > > > > original patch it would look like this: > > foobar: Adjusted the foo setting in bar > > > > When using foobar on systems with less than a gigabyte of RAM common > > usage patterns often result in an Out-of-memory condition causing > > slowdowns and unexpected application termination. > > > > Low-memory systems should continue to function without running into > > memory-starvation conditions with minimal cost to systems with more > > available memory. High-memory systems will be less able to use the > > full extent of the system, a dynamically tunable option may be best, > > long-term. > > > > The foo setting in bar was decreased from X to X-50% in order to > > ensure we don't exhaust all system memory with foobar threads. > > > > Signed-off-by: Joe Developer <[email protected]> > > > > The foobar recipe was imported from the OpenEmbedded git server > > (git://git.openembedded.org/openembedded) as of commit id > > b65a0e0c84cf489bfa00d6aa6c48abc5a237100f. > > > > It was necessary to change the memory threshold from 50% to 42% > > due to the constraints of the implementation of bar in OpenEmbedded. > > > > Signed-off-by: Your Name <[email protected]> > > > > Common Errors in Patch and Commit messages > > ------------------------------------------ > > > > - Don't have one huge patch, split your change into logical subparts. > > It's easier to track down problems afterwards with a binary search and > > also people can then cherrypick changes into things like stable > > branches. > > > > - Don't simply translate your change into English for a commit log. The > > log "Change compare from zero to one" is bad because it describes only > > the code change in the patch; we can see that from reading the patch > > itself. Let the code tell the story of the mechanics of the change (as > > much as possible), and let your comment tell the other details -- i.e. > > what the problem was, how it manifested itself (symptoms), and if > > necessary, the justification for why the fix was done in manner that it > > was. > > > > - Don't start your commit log with "This patch..." -- we already know it > > is a patch. > > > > - Don't repeat your short log in the long log. If you really really don't > > have anything new and informational to add in as a long log, then don't > > put a long log at all. This should be uncommon -- i.e. the only > > acceptable cases for no long log would be something like "Fix spelling > > mistakes in Documentation/README" or similar. > > > > - Don't put absolute paths to source files that are specific to your > > site, i.e. if you get a compile error on "fs/exec.c" then tidy up the > > parts of the compile output to just show that portion. We don't need to > > see that it was /home/wally/src/git/oe-core/meta/classes/insane.bbclass > > or similar - that full path has no value to others. > > > > - Always use the most significant ramification of the change in the words > > of your subject/shortlog. For example, don't say "fix compile warning in > > foo" when the compiler warning was really telling us that we were > > dereferencing complete garbage off in the weeds that could in theory > > cause an OOPS under some circumstances. When people are choosing commits > > for backports to stable or distro kernels, the shortlog will be what > > they use for an initial sorting selection. If they see "Fix possible > > OOPS in...." then these people will look closer, whereas they most > > likely will skip over simple warning fixes. > > > > - Don't put in the full 20 or more lines of a backtrace when really it is > > just the last 5 or so function calls that are relevant to the crash/fix. > > If the entry point, or start of the trace is relevant (i.e. a syscall or > > similar), you can leave that, and then replace a chunk of the > > intermediate calls in the middle with a single [...] > > > > - Don't include timestamps or other unnecessary information, unless they > > are relevant to the situation (i.e. indicating an unacceptable delay in > > a driver initialization etc.) > > > > - Don't use links to temporary resources like pastebin and friends. The > > commit message may be read long after this resource timed out. > > > > - Don't reference mirrors, but instead always reference original > > authoritative sources. > > > > - Avoid punctuation in the short log. How was the patch tested - Intentionally unmentioned?
Andreas _______________________________________________ Openembedded-devel mailing list [email protected] http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
