Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
Hello Mark, On Thu, May 12, 2011 at 9:57 PM, Mark Hatle mark.ha...@windriver.com wrote: ... 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. In other words, the long commit message must describe *why* the change was needed (instead of what has changed). I find this item the most lacking in current OpenEmbedded/Poky related commits. Thanks for working on this, -- Leon ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
Hi Mark, Thanks for writing this up. I think this is on the right track. A few comments inline... As a side note, I'm currently working with various people on improving our review/pull process which dovetails with this a bit, but probably doesn't need to be explicitly called out here. On 05/12/2011 12:57 PM, Mark Hatle wrote: This is the fifth draft of the guidelines... All previous feedback has been incorporated... (The fourth draft was reviewed and found lacking in a few key areas, so I skipped the public posting.) The new concept of Upstream-Status was added by the fourth draft as something strongly suggested, but optional. This is likely an area that may still need some revision before we call this final. Introduction This set of guidelines is intended to help both the developer and reviewers of changes determine reasonable patch headers 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 header 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. This policy refers both to patches that are being applied by recipes as well as commit messages that are part of the source control system, usually git. A patch file needs a header in order to describe the specific change that that are made within that patch, while a commit message describes one or more changes to the overall project or system. Both the patch headers and commit messages require the same attention and basic details, however the purposes of the messages are slightly different. A commit message documents all of the changes made as part of a commit, while a patch header documents items specific to a single patch. In many cases the patch header can also be used as the commit message. This policy does not cover the testing of the changes, or the technical criteria for accepting a patch. 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. The 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! In addition section 5.4 explains the purpose of the Signed-off-by: tag line as discussed in later parts of this document. Due to the importance of the Signed-off-by: tag line a copy of the description follows: Signed-off-by: this is a developer's certification that he or she has the right to submit the patch for inclusion into the [project]. It is an agreement to the Developer's Certificate of Origin, the full text of which can be found in [Linux Kernel] Documentation/SubmittingPatches. Code without a proper signoff cannot be merged into the mainline. For more information on the Developer's Certificate of Origin and the Linux Kernel Documentation/SubmittingPatches, see: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches Patch Headers and Commit Messages - 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 such as Signed-off-by:. A complete list of acceptable tag lines should be documented. Consider: Signed-off-by: Acked-by:\__ The difference between these two is subtle. Acked-by Reviewed-by: / implies a certain degree of authority over the affected Tested-by: code. Reported-by: 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:
Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
On 5/12/11 4:00 PM, Darren Hart wrote: Hi Mark, Thanks for writing this up. I think this is on the right track. A few comments inline... As a side note, I'm currently working with various people on improving our review/pull process which dovetails with this a bit, but probably doesn't need to be explicitly called out here. On 05/12/2011 12:57 PM, Mark Hatle wrote: This is the fifth draft of the guidelines... All previous feedback has been incorporated... (The fourth draft was reviewed and found lacking in a few key areas, so I skipped the public posting.) The new concept of Upstream-Status was added by the fourth draft as something strongly suggested, but optional. This is likely an area that may still need some revision before we call this final. Introduction This set of guidelines is intended to help both the developer and reviewers of changes determine reasonable patch headers 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 header 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. This policy refers both to patches that are being applied by recipes as well as commit messages that are part of the source control system, usually git. A patch file needs a header in order to describe the specific change that that are made within that patch, while a commit message describes one or more changes to the overall project or system. Both the patch headers and commit messages require the same attention and basic details, however the purposes of the messages are slightly different. A commit message documents all of the changes made as part of a commit, while a patch header documents items specific to a single patch. In many cases the patch header can also be used as the commit message. This policy does not cover the testing of the changes, or the technical criteria for accepting a patch. 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. The 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! In addition section 5.4 explains the purpose of the Signed-off-by: tag line as discussed in later parts of this document. Due to the importance of the Signed-off-by: tag line a copy of the description follows: Signed-off-by: this is a developer's certification that he or she has the right to submit the patch for inclusion into the [project]. It is an agreement to the Developer's Certificate of Origin, the full text of which can be found in [Linux Kernel] Documentation/SubmittingPatches. Code without a proper signoff cannot be merged into the mainline. For more information on the Developer's Certificate of Origin and the Linux Kernel Documentation/SubmittingPatches, see: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches Patch Headers and Commit Messages - 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 such as Signed-off-by:. A complete list of acceptable tag lines should be documented. Consider: Signed-off-by: Acked-by:\__ The difference between these two is subtle. Acked-by Reviewed-by: / implies a certain degree of authority over the affected Tested-by: code. Reported-by: These were in the original guidelines, but it was decided by the OE-TSC, and general comments from the OE users/developers that Signed-off-by is all that is needed. Anything beyond this is acceptable, but not part of the overall guidlines. New development --- A minimal patch or commit message would be of the format: Short log / Statement of
Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
One more thing. There is a great deal of cross-posting to the various lists. We should discourage this. At least mentioning somewhere in here that patches should be sent to the appropriate list with maintainers and involved developers on CC and not cross posted would be a help. -- Darren On 05/12/2011 02:00 PM, Darren Hart wrote: Hi Mark, Thanks for writing this up. I think this is on the right track. A few comments inline... As a side note, I'm currently working with various people on improving our review/pull process which dovetails with this a bit, but probably doesn't need to be explicitly called out here. On 05/12/2011 12:57 PM, Mark Hatle wrote: This is the fifth draft of the guidelines... All previous feedback has been incorporated... (The fourth draft was reviewed and found lacking in a few key areas, so I skipped the public posting.) The new concept of Upstream-Status was added by the fourth draft as something strongly suggested, but optional. This is likely an area that may still need some revision before we call this final. Introduction This set of guidelines is intended to help both the developer and reviewers of changes determine reasonable patch headers 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 header 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. This policy refers both to patches that are being applied by recipes as well as commit messages that are part of the source control system, usually git. A patch file needs a header in order to describe the specific change that that are made within that patch, while a commit message describes one or more changes to the overall project or system. Both the patch headers and commit messages require the same attention and basic details, however the purposes of the messages are slightly different. A commit message documents all of the changes made as part of a commit, while a patch header documents items specific to a single patch. In many cases the patch header can also be used as the commit message. This policy does not cover the testing of the changes, or the technical criteria for accepting a patch. 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. The 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! In addition section 5.4 explains the purpose of the Signed-off-by: tag line as discussed in later parts of this document. Due to the importance of the Signed-off-by: tag line a copy of the description follows: Signed-off-by: this is a developer's certification that he or she has the right to submit the patch for inclusion into the [project]. It is an agreement to the Developer's Certificate of Origin, the full text of which can be found in [Linux Kernel] Documentation/SubmittingPatches. Code without a proper signoff cannot be merged into the mainline. For more information on the Developer's Certificate of Origin and the Linux Kernel Documentation/SubmittingPatches, see: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches Patch Headers and Commit Messages - 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 such as Signed-off-by:. A complete list of acceptable tag lines should be documented. Consider: Signed-off-by: Acked-by:\__ The difference between these two is subtle. Acked-by Reviewed-by: / implies a certain degree of authority over the affected Tested-by: code. Reported-by: New development --- A minimal patch or commit message
Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
On 05/12/2011 05:11 PM, Darren Hart wrote: One more thing. There is a great deal of cross-posting to the various lists. We should discourage this. At least mentioning somewhere in here that patches should be sent to the appropriate list with maintainers and involved developers on CC and not cross posted would be a help. Ok, so I am going to go ahead and ask the dumb question. Where is the write up that say what list is for what? It seems to me there is a whole bunch of discussion happening on the poky list about making changes that that would effect everyone using openembedded-core. Do we have two lists for discussion of stuff that effect the oe-core? -- Bill ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
On 05/12/2011 03:05 PM, William Mills wrote: On 05/12/2011 05:11 PM, Darren Hart wrote: One more thing. There is a great deal of cross-posting to the various lists. We should discourage this. At least mentioning somewhere in here that patches should be sent to the appropriate list with maintainers and involved developers on CC and not cross posted would be a help. Ok, so I am going to go ahead and ask the dumb question. Where is the write up that say what list is for what? Not a dumb question at all! Unfortunately, I think a lot of people are confused about this, which is natural as much of this separation is still new to people. There is: http://www.yoctoproject.org/community/mailing-lists Which needs to be updated to better reflect the oe-core aspect I think. And: http://openembedded.org/index.php/Mailing_lists This accurately describes oe-core (and other oe specific lists). It seems to me there is a whole bunch of discussion happening on the poky list about making changes that that would effect everyone using openembedded-core. Do we have two lists for discussion of stuff that effect the oe-core? No. As I understand it, things that go into poky.git/meta and things that go into oe-core should be sent to the openembedded-core list. Patch series for poky.git should be isolated in such a way so that they do not include changes to meta (oe-core) and other areas at the same time. Does anyone have a different view? -- Bill -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
On 05/12/2011 07:49 PM, Darren Hart wrote: There is: http://www.yoctoproject.org/community/mailing-lists Which needs to be updated to better reflect the oe-core aspect I think. This might be at least one source of the cross posting as people are not sure poky vs oe-core. It seems to me there is a whole bunch of discussion happening on the poky list about making changes that that would effect everyone using openembedded-core. Do we have two lists for discussion of stuff that effect the oe-core? No. As I understand it, things that go into poky.git/meta and things that go into oe-core should be sent to the openembedded-core list. Patch series for poky.git should be isolated in such a way so that they do not include changes to meta (oe-core) and other areas at the same time. Does anyone have a different view? This seems logical to me. Thanks. I also think discussion of a potential patch should be done on the list that the patch would go to yes? Maybe we could drive that definition to ground and get the mailing list page updated. --- So above, perhaps my whole bunch was overstated. Let me retract that and say at least some. I don't mean to pick on you Darren; I really am just trying to understand and an example is often helpful. Earlier you and Richard were discussing directdisc images [1] with a suggestion that it might be ripe for removal. Should that have been on oe-core? [1] https://lists.yoctoproject.org/pipermail/poky/2011-May/006106.htm Thanks, Bill ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] [poky] Commit and Patch message guidelines - fifth draft
On 05/12/2011 07:03 PM, William Mills wrote: On 05/12/2011 07:49 PM, Darren Hart wrote: There is: http://www.yoctoproject.org/community/mailing-lists Which needs to be updated to better reflect the oe-core aspect I think. This might be at least one source of the cross posting as people are not sure poky vs oe-core. It seems to me there is a whole bunch of discussion happening on the poky list about making changes that that would effect everyone using openembedded-core. Do we have two lists for discussion of stuff that effect the oe-core? No. As I understand it, things that go into poky.git/meta and things that go into oe-core should be sent to the openembedded-core list. Patch series for poky.git should be isolated in such a way so that they do not include changes to meta (oe-core) and other areas at the same time. Does anyone have a different view? This seems logical to me. Thanks. I also think discussion of a potential patch should be done on the list that the patch would go to yes? Maybe we could drive that definition to ground and get the mailing list page updated. --- So above, perhaps my whole bunch was overstated. Let me retract that and say at least some. I don't mean to pick on you Darren; Oh, I think I asked for this one :-) I really am just trying to understand and an example is often helpful. Earlier you and Richard were discussing directdisc images [1] with a suggestion that it might be ripe for removal. Should that have been on oe-core? You are correct. I should have sent that to oe-core as the implementation in question is in poky.git/meta (ie oe-core). Clearly, this is easy to miss and will take some time for us to get there. Thanks for pointing that out! [1] https://lists.yoctoproject.org/pipermail/poky/2011-May/006106.htm Thanks, Bill -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
