Changes between Version 15 and Version 16 of DeveloperDocumentation


Ignore:
Timestamp:
10/31/23 12:48:23 (5 months ago)
Author:
flichtenheld
Comment:

Typo and grammar fixes

Legend:

Unmodified
Added
Removed
Modified
  • DeveloperDocumentation

    v15 v16  
    2222 * All patches must be sent to "openvpn-devel" mailing list for review.  The subject should preferably be prefixed with '''[PATCH]'''
    2323 * All patches need to be reviewed and accepted (ACK). The ACK process is logically split into two (see below).
    24  * All accepted patches go to the OpenVPN "master" branch (Git)
     24 * All accepted patches go to the OpenVPN '''master''' branch (Git)
    2525
    2626The author must send patches to the devel mailing list.  This is so to open up for a public discussion of the changes, and to allow the ACK process to work.  All patches going into the tree will contain explicit Acked-by: references, to document who gave the approval.
     
    3030 * You can see a list of replies to a patch including the ACKs in [https://patchwork.openvpn.net/project/openvpn2/list/ Patchwork].
    3131 * Patches sent ''directly'' to a development tree maintainer will be rejected
    32  * GitHub pull requests can be useful with large patchsets, or if you want to gauge interest in your patch. The final patch needs to go to "openvpn-devel" mailing list, however.
     32 * !GitHub pull requests can be useful with large patchsets, or if you want to gauge interest in your patch. The final patch needs to go to "openvpn-devel" mailing list, however.
    3333
    3434== Adding placeholder bug reports ==
     
    7070
    7171A 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. 
    72 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.
    73 
    74 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. 
    75 
    76 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. 
    77 
    78 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.
     72This 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 advice 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.
     73
     74To explain the Signed-off-by and Acked-by process a bit further.  In git you have two "data fields" which are populated automatically when commits are done, author and committer.  The committer 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. 
     75
     76Then you have the author field which is usually 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. 
     77
     78If the author information is present, the sender field will be treated differently as well, in those cases where someone sends a patch on behalf of somebody else.
    7979
    8080So to summarize, we track these fields almost automatically:
     
    8383 * committer - information about who ''applied'' the patch to the git tree
    8484
    85 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).
    86 
    87 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.
     85When 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'').
     86
     87The one who receives the patch and finds it good to go even further (if you want to 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 has been looking at the patch.  And the more people who have eye-balled the patch and added their ''Signed-off'' line to the patch, the better!  By doing so, the reviewer indicates that the patch is good.
    8888
    8989Before it goes into the tree, a final "Acked-by:" note is added, indicating who accepted it for inclusion into the tree,
     
    123123}}}
    124124
    125 '''When Bob Boss ACK's the patch''', the final commit to be found in the
     125'''When Bob Boss ACKs the patch''', the final commit to be found in the
    126126source code repository will be:
    127127{{{
     
    142142}}}
    143143
    144 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).
    145 
    146 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. 
    147 
    148 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.
    149 
    150 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.
     144The Acked-by: can be the one who does the final commit to the source code repository, but it can also be someone who doesn't do the final commit.  The last Signed-off-by should normally be the person who does the final commit.  This is usually the person  who "touched" the patch by adding the Acked-by line(s).
     145
     146However, you should not see any commits which are written by (author) and Acked-by by the same person.  Then the process has failed to give a qualified review. 
     147
     148You may ask why we have this "bureaucracy" (which is a fair question!).  It is simply to keep all who send in patches, those who review them and finally accept 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.
    151149
    152150= Handling security issues =
     
    166164 * Must be sent as unified diff (diff -u) or preferably generated by ''git format-patch''
    167165 * Patches must have a short but descriptive one-line summary of the change, followed by a more descriptive text explaining in plain English why and how the patch is written.  This description should ''not'' be too technical, as the patch itself will reveal technical details.  More on [http://who-t.blogspot.com/2009/12/on-commit-messages.html writing good commit messages here]
    168  * Patches should contain a 'Signed-off-by:' line at the end (git commit --signoff).  If missing, it will be added with the sender of the patch as the signed-off person.
     166 * Patches should contain a `Signed-off-by:` line at the end (''git commit --signoff'').  If missing, it will be added with the sender of the patch as the signed-off person.
    169167 * Patches must apply cleanly, without merge conflicts.  Please state which code base the patch is written against.  If based on the git tree, please state which branch it is to be applied to.
    170168 * Everyone who has contributed to this patch should be mentioned out of courtesy and respect to all contributors and helpers on the way.   If suitable valid e-mail address should be used, preferably in addition with full name.
     
    189187
    190188 * [http://mywiki.wooledge.org/Bashism Bashism - Greg's Wiki]
    191  * [http://sourceforge.net/projects/checkbaskisms/ Checkbashisms]: a tool to check for "Bashisms". Also included in Debian/Ubuntu "devscripts" package.
     189 * [http://sourceforge.net/projects/checkbaskisms/ Checkbashisms]: a tool to check for "Bashisms". Also included in !Debian/Ubuntu "devscripts" package.
    192190 * [http://gondor.apana.org.au/~herbert/dash DASH]: a minimal shell with "POSIX-compliant features only", probably useful for testing
    193191