Dear Mark,
I forgot to add one thing. Am Donnerstag, den 31.03.2011, 21:06 +0200 schrieb Paul Menzel: > Am Donnerstag, den 31.03.2011, 12:57 -0500 schrieb Mark Hatle: > > 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. > > No space in OpenEmbedded. ;-) > > > 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, > > Excuse my ignorance and please keep in mind that English is not my > native language. I do not get that sentence. How can a patch describe > something. > > Reading further down, I now get in what context you use patch here. > Maybe you should make clear in the beginning that you mean patches which > gets applied by the recipe and not patches to be submitted directly to > OpenEmbedded. (That is still ambiguous.) > > > 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. > > s/They/The/ > > > 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. > > 1. I think that line is also named »commit summary« in Git terms. > 2. Do we suggest a maximum line length for the short log? > > > (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 <joe.develo...@example.com> > > > > 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. > > Example. > > > 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. > > Is a tag line also needed then? What kind of tag line? > > > 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. > > Are the tag lines also needed in the patch? > > > 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 <joe.develo...@example.com> > > > > The foobar recipe was imported from the OpenEmbedded git server > > (git://git.openembedded.org/openembedded) as of commit id > > b65a0e0c84cf489bfa00d6aa6c48abc5a237100f. > > > > Integrated-by: Your Name <your.n...@openembedded.org> > > > > The above example shows a clear way for a developer to find the original > > source > > of the change. It indicates that the open embedded > > s/open embedded/OpenEmbedded/ > > > 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 <joe.develo...@example.com> > > > > 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 <your.n...@openembedded.org> > > > > > > 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 > > Mention `git bisect`? > > > and also people can then cherrypick > > s/cherrypick/cherry pick/? > > > 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. > > A common case is also when updating/adding program versions. > > foo: add version 10.μ.β.π > > > - 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. > > How much should it be stripped? > > > - 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 [...] > > I have to remember that. ;-) > > > - 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. > > I would then also propose to *not* start with a capital letter. > > In OpenEmbedded it is common to write the recipe name (and if necessary > the version) in front of the commit summary. I kind of liked that rule. Additionally for quality assurance I would ask to add a line what distribution and `MACHINE` the patch was build tested (or even run tested) with. Adding the output »Build configuration« of BitBake should be sufficient. > Thank you for the write up/proposal. All in all it is quite long. Maybe > a summary/short version/example should be put right at the beginning. Thanks, Paul
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel