| 31 | | == Community patches and the acceptance process of these patches == |
| 32 | | |
| 33 | | All patches needs to receive an ACK (Acknowledged) before being accepted in to the Git repository. To encourage participation from non-developers the ACK is split into two parts: |
| 34 | | |
| 35 | | * ''"This patch does the right thing ACK"''. Also known as ''feature ACK''. One does not have to be a developer to give this ACK. |
| 36 | | * ''"This patch does the thing (it does) right"''. Also known as ''code quality ACK''. People who understand the code can give this. |
| 37 | | |
| 38 | | Note that both ACKs can be given by a single person. |
| 39 | | |
| 40 | | A patch has to get both ACKs to be included into the development tree. The ACK process is used to ensure that only useful and high-quality code gets into OpenVPN. Hopefully that will make it easier for those who don't feel they are "good enough" developers to try to send patches anyway. |
| 41 | | This whole review process is to give and receive feedback, where we as a community can help making OpenVPN better - together. For example, the more experienced developers can share their advices to those who are less experienced. Those being less experienced can share their understanding of the code as well, which might shed some light to issues or solutions which are better. Non-developers can also participate by sharing their expertise on OpenVPN in general by giving feature ACKs (NACKs) when applicable. This way everyone can learn something and hopefully even get wiser. |
| 42 | | |
| 43 | | To explain the Signed-off-by and Acked-by process a bit further. In git you have to "data fields" which are populated automatically when commits are done, author and commiter. The commiter will be the one who applies patches from the mailing list or the one doing commits to a tree which are fetched via the remote access features in git. |
| 44 | | |
| 45 | | Then you have the author field which is either present in the patch file itself. This field might be missing if ''git format-patch'' has not been used to create the patch file sent to the mailing list. In these cases, it is expected that the author of the patch is the same as the sender - unless the commit message in the patch indicates something else. |
| 46 | | |
| 47 | | If the author information is present, the sender field will be treated differently as wel, in those cases where someone sends a patch on behalf of somebody else. |
| 48 | | |
| 49 | | So to summarize, we track these fields almost automatically: |
| 50 | | * author - information about the person who ''wrote'' the patch |
| 51 | | * sender - information about who ''sent / forwarded'' the patch for inclusion |
| 52 | | * committer - information about who ''applied'' the patch to the git tree |
| 53 | | |
| 54 | | Then a patch author sends his patch to somewhere, he should make sure it contains a 'Signed-off-by: {Full name} <email@example.com>' line in the commit text. This is to officially announce that declare that this patch is ready for further processing. When doing git commits, you can add this indication automatically by doing 'git commit -s' (or --signed-off). |
| 55 | | |
| 56 | | The one who receives the patch and finds it good to go even further (if want you do some kind of "internal" review before sending it further to the public) will add his/hers "Signed-off-by" reference as well when sending further again. This way we can track who have been looking at the patch. And the more people who have eye-balled the patch and adds their "Signed-off" line to the patch, the better! By doing so, the reviewer indicates that the patch is good. |
| 57 | | |
| 58 | | Before it goes into the tree, a final "Acked-by:" note is added, indicating who accepted it for inclusion into the tree, |
| 59 | | |
| 60 | | === Example of patch submission and the review process === |
| 61 | | |
| 62 | | Lets say John Doe writes a patch, sends it to Jane Doe and she sends it |
| 63 | | Bob Boss for inclusion. The process would be something like this: |
| 64 | | |
| 65 | | '''John Doe:''' |
| 66 | | {{{ |
| 67 | | \tAuthor: John Doe <john.doe@example.com> |
| 68 | | |
| 69 | | Short summary of the patch |
| 70 | | |
| 71 | | \tA more detailed description of this patch fixing x.y.z. This can |
| 72 | | even span over several lines to explain the patch. |
| 73 | | |
| 74 | | \tSigned-off-by: John Doe <john.doe@example.com> |
| 75 | | |
| 76 | | \t<the patch itself> |
| 77 | | }}} |
| 78 | | |
| 79 | | '''Jane Doe will send it further as:''' |
| 80 | | {{{ |
| 81 | | \tAuthor: John Doe <john.doe@example.com> |
| 82 | | |
| 83 | | Short summary of the patch |
| 84 | | |
| 85 | | \tA more detailed description of this patch fixing x.y.z. This can |
| 86 | | even span over several lines to explain the patch. |
| 87 | | |
| 88 | | \tSigned-off-by: John Doe <john.doe@example.com> |
| 89 | | \tSigned-off-by: Jane Doe <jane@doe.net> |
| 90 | | |
| 91 | | \t<the patch itself> |
| 92 | | }}} |
| 93 | | |
| 94 | | '''When Bob Boss ACK's the patch''', the final commit to be found in the |
| 95 | | source code repository will be: |
| 96 | | {{{ |
| 97 | | \tAuthor: John Doe <john.doe@example.com> |
| 98 | | |
| 99 | | Short summary of the patch |
| 100 | | |
| 101 | | \tA more detailed description of this patch fixing x.y.z. This can |
| 102 | | even span over several lines to explain the patch. |
| 103 | | |
| 104 | | \tSigned-off-by: John Doe <john.doe@example.com> |
| 105 | | \tSigned-off-by: Jane Doe <jane@doe.net> |
| 106 | | \tAcked-by: Bob Boss <bob@bigbucks.com> |
| 107 | | \tSigned-off-by: David Sommerseth <dazo@users.sourceforge.net> |
| 108 | | |
| 109 | | |
| 110 | | \t<the patch itself> |
| 111 | | }}} |
| 112 | | |
| 113 | | The Acked-by: can be the one who does the final commit to the source code repository, but it can also be someone who don't do the final commit. The last Signed-off-by should normally be the person the one who does the final commit. This is usually the person who "touched" the patch by adding the Acked-by line(s). |
| 114 | | |
| 115 | | However, you should not see any commits which is written by (author) and Acked-by by the same person. Then the process have failed to give a qualified review. |
| 116 | | |
| 117 | | You may ask why we have this "bureaucracy" (which is a fair question!). It is simply to keep all who send in patches, those who reviews them and finally accepts them into the source code repository more accountable for their work. This process will document each accepted patch and give an indication that it has been through a certain set of reviews. This is to ensure that we don't accept bad code easily into the source code repository. This is just to keep this project as transparent and open as possible. |
| 118 | | |
| 119 | | For now, the only patches which which "breaks" this process in the git tree are the patches coming in from James' SVN BETA21 branch. But we trust that James tracks the patches he pulls in according to the standards he needs to feel comfortable with the OpenVPN code. |
| 120 | | |
