diff mbox

contributing: add user agreement and short log conventions

Message ID 1465833895-14871-1-git-send-email-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer June 13, 2016, 4:04 p.m. UTC
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 CONTRIBUTING | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 131 insertions(+), 2 deletions(-)

Comments

Mike Holmes June 14, 2016, 11:50 a.m. UTC | #1
On 13 June 2016 at 12:04, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
>  CONTRIBUTING | 133
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 131 insertions(+), 2 deletions(-)
>
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> index a81fd8d..b0b2781 100644
> --- a/CONTRIBUTING
> +++ b/CONTRIBUTING
> @@ -11,12 +11,27 @@ in understanding the  contributing requirements for ODP
>  This document is intended to guide a new application developer in
> understanding
>  the  contributing requirements for ODP
>
> +== Contributor's Agreement
> +ODP is an open source project that follows the
> +https://opensource.org/licenses/BSD-3-Clause[BSD 3 Clause] license. Every
> +contributor to ODP must agree to comply by these licensing terms as well
> as
> +assert that they have the right to contribute the code they submit to the
> +project. No patches will be considered for acceptance without the
> contributor
> +having first executed either an
> +http://www.opendataplane.org/contributor/individual/[Individual
> Contributor
> +License Agreement] or being a member of an orgnaization that has executed
> +a http://www.opendataplane.org/contributor/corporate/[Corporate
> Contributor
> +License Agreement].
> +
> +These agreements are a one-time requirement and take only a few minutes to
> +perform by click-through.
> +
>  == New Development
>
>  ODP code shall be written with the kernel coding style
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle[Kernel
> Coding Style]
>
>  ODP code shall be documented using the doxygen style described in the
> -"Documenting the code" section.
> +<<Documenting the Code>> section.
>  Check patch script/checkpatch.pl shall be used before submitting a patch.
>
>  == ODP patch expectations as an  open source project
> @@ -42,6 +57,120 @@ which can be found in
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linu
>
>  Code without a proper signoff cannot be merged into the mainline.
>
> +== Short Log and Long Log Conventions
> +For ease of patch management, every submitted patch must have a short log
> +entry. The short log is a one-line summary of the patch and says where it
> +is intended to be applied and (briefly) its purpose. Following the short
> +log, submitters are free to add additional detail in the long log. Long
> log
> +entries are encouraged when appropriate, but there are no specific
> requirements
>


I think there are some details we expect to the point of being requirements
and we should re refer them to the prior section as we state in the section
"
ODP patch expectations as an open source project" some specific  guidelines
by refering to
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches
and section two "Describe your changes" is resonable summary of a good
patch description.


> +governing their structure or contents other than that they be relvant and
> +useful to potential reviewers in understanding the purpose and rationale
> +for the patch.

+
> +=== Short log format
> +ODP requires that the short log be a single line not exceeding 80
> characters,
> +be composed entirely in lower case, and be structured as follows:
> +
> +`area: component: brief description`
> +
> +Note that a single colon (:) should be used to separate each section of
> the
> +short log and be followed by a single space before beginning the next
> section.
> +
> +Area identifies the functional area that this patch addresses and in
> general
> +says where in the directory hierarchy the patch is intended to apply. This
> +should be taken from the following list:
> +
> +api::
> +Patch adds, deletes, or changes an ODP API. Any patch that applies to the
> +`include` directory or its descendants is considered an API patch and must
> +be identified as such. Moreover, any patch series containing an api patch
> +must be identified in the patch's `--subject-prefix` as API-NEXT.

+
> +doc::
> +Patch applies to the ODP documentation library contained in the `doc`
> directory
> +
> +example::
> +Patch applies to the ODP examples contained in the `example` directory.
> +
> +helper::
> +Patch applies to the ODP helper library contained in the `helper`
> directory.
> +
> +performance::
> +Patch applies to the ODP performance test suite contained in the
> +`test/performance` directory.
> +
> +linux-generic::
> +linux-gen::
> +l-g::
>

This last identifier cannot be as easily grepped for in a single grep of
 "linux-gen" I dont say we have to drop it, but it feels a little less
consistent


> +Patch applies to the odp-linux reference implementation, specifically the
> +`platform/linux-generic/` directory that contains that implementation. The
> +alternate forms `linux-gen` or `l-g` may be used to conserve line space if
> +desired.
> +
> +validation::
> +Patch applies to the ODP validation test suite contained in the
> +`test/validation` directory.
> +
> +Any other patch that applies to some other top-level directory or file
> should
> +use that directory or file name as its area.
> +
> +The component portion of the short log identifies the ODP component that
> this
> +patch applies to. A component is required unless the area is a single
> file, in
> +which case it may be omitted. Components should be selected from the
> following
> +list:
> +
> +buffer::
> +Buffers and buffer processing.
> +
> +classification::
> +cls::
>

class would make for a simple grep if  we accept an alternative short form


> +Classification. `cls` may be used as an abbreviation for space if desired.
> +
> +crypto:
> +Crypto.
> +
> +pktio::
> +Packet I/O (pktio) and its related sub-components
> +
> +packet::
> +Packets and packet processing.
> +
> +pool::
> +Buffer pools and related processing.
> +
> +queue::
> +Queues and related processing.
> +
> +scheduler::
> +sched::
> +Scheduler and related processing.
> +
> +time::
> +Time.
> +
> +timer::
> +Timers and related processing.
> +
> +tm::
> +Traffic Manager and related processing.
> +
> +Following the component the rest of the short log should describe the
> purpose
> +of this patch.
> +
> +=== Examples
> +To illustrate, here are some examples of good and bad short logs:
> +
> +Good::
> +* `api: pool: add new foo attribute to pools`
> +* `linux-generic: pool: implement new foo attribute for pools`
> +* `validation: pool: test new foo attribute for pools`
> +
> +Bad::
> +* `fix something` (doesn't follow required format)
> +* `api: propose a new api` (missing component)
> +* `validation: pool: a description that rambles on and on and never gets
> to
> +the point`
> +
>  == Common Errors in Patch and Commit Messages
>
>  - Avoid starting the summary line with a capital letter, unless the
> component
> @@ -92,7 +221,7 @@ Code without a proper signoff cannot be merged into the
> mainline.
>
>  - References to wikipedia are not permitted.
>
> -=== Documenting the code
> +=== Documenting the Code
>
>  - Allow doxygen to use all its default behaviors to identify tagged
>    information but where a doxygen tag must be specified use @
> --
> 2.7.4
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov June 14, 2016, 12:40 p.m. UTC | #2
On 06/14/16 15:30, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>> +
>> +performance::
>> +Patch applies to the ODP performance test suite contained in the
>> +`test/performance` directory.
> Performance is not optimal tag since it gives an impression that some performance optimization was done, etc.
>
> I have used e.g. "test: l2fwd: ..."
>
>
>
>> +
>> +linux-generic::
>> +linux-gen::
>> +l-g::
>> +Patch applies to the odp-linux reference implementation, specifically the
>> +`platform/linux-generic/` directory that contains that implementation.
>> The
>> +alternate forms `linux-gen` or `l-g` may be used to conserve line space
>> if
>> +desired.
> l-g is may be too ambiguous - especially when the commit is applied to another implementation/repo (where l-g may have another meaning).

I think we should use only 1 variant of prefix. That will allow us to 
filter and sort list of changes.

Maxim.

>
>> +
>> +validation::
>> +Patch applies to the ODP validation test suite contained in the
>> +`test/validation` directory.
>> +
> I have used simply "test: sched: ...", "test: pool: ...", etc
>
> Most of the test directory content is validation tests anyway. 'Make check' runs also some performance tests. Examples may be distinguished with "example: ".
>
>
> -Petri
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Bill Fischofer June 27, 2016, 4:16 p.m. UTC | #3
On Tue, Jun 14, 2016 at 6:50 AM, Mike Holmes <mike.holmes@linaro.org> wrote:

>
>
> On 13 June 2016 at 12:04, Bill Fischofer <bill.fischofer@linaro.org>
> wrote:
>
>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
>> ---
>>  CONTRIBUTING | 133
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 131 insertions(+), 2 deletions(-)
>>
>> diff --git a/CONTRIBUTING b/CONTRIBUTING
>> index a81fd8d..b0b2781 100644
>> --- a/CONTRIBUTING
>> +++ b/CONTRIBUTING
>> @@ -11,12 +11,27 @@ in understanding the  contributing requirements for
>> ODP
>>  This document is intended to guide a new application developer in
>> understanding
>>  the  contributing requirements for ODP
>>
>> +== Contributor's Agreement
>> +ODP is an open source project that follows the
>> +https://opensource.org/licenses/BSD-3-Clause[BSD 3 Clause] license.
>> Every
>> +contributor to ODP must agree to comply by these licensing terms as well
>> as
>> +assert that they have the right to contribute the code they submit to the
>> +project. No patches will be considered for acceptance without the
>> contributor
>> +having first executed either an
>> +http://www.opendataplane.org/contributor/individual/[Individual
>> Contributor
>> +License Agreement] or being a member of an orgnaization that has executed
>> +a http://www.opendataplane.org/contributor/corporate/[Corporate
>> Contributor
>> +License Agreement].
>> +
>> +These agreements are a one-time requirement and take only a few minutes
>> to
>> +perform by click-through.
>> +
>>  == New Development
>>
>>  ODP code shall be written with the kernel coding style
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle[Kernel
>> Coding Style]
>>
>>  ODP code shall be documented using the doxygen style described in the
>> -"Documenting the code" section.
>> +<<Documenting the Code>> section.
>>  Check patch script/checkpatch.pl shall be used before submitting a
>> patch.
>>
>>  == ODP patch expectations as an  open source project
>> @@ -42,6 +57,120 @@ which can be found in
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linu
>>
>>  Code without a proper signoff cannot be merged into the mainline.
>>
>> +== Short Log and Long Log Conventions
>> +For ease of patch management, every submitted patch must have a short log
>> +entry. The short log is a one-line summary of the patch and says where it
>> +is intended to be applied and (briefly) its purpose. Following the short
>> +log, submitters are free to add additional detail in the long log. Long
>> log
>> +entries are encouraged when appropriate, but there are no specific
>> requirements
>>
>
>
> I think there are some details we expect to the point of being
> requirements and we should re refer them to the prior section as we state
> in the section "
> ODP patch expectations as an open source project" some specific
>  guidelines by refering to
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches
> and section two "Describe your changes" is resonable summary of a good
> patch description.
>

OK, will add this to v2.


>
>> +governing their structure or contents other than that they be relvant and
>> +useful to potential reviewers in understanding the purpose and rationale
>> +for the patch.
>
> +
>> +=== Short log format
>> +ODP requires that the short log be a single line not exceeding 80
>> characters,
>> +be composed entirely in lower case, and be structured as follows:
>> +
>> +`area: component: brief description`
>> +
>> +Note that a single colon (:) should be used to separate each section of
>> the
>> +short log and be followed by a single space before beginning the next
>> section.
>> +
>> +Area identifies the functional area that this patch addresses and in
>> general
>> +says where in the directory hierarchy the patch is intended to apply.
>> This
>> +should be taken from the following list:
>> +
>> +api::
>> +Patch adds, deletes, or changes an ODP API. Any patch that applies to the
>> +`include` directory or its descendants is considered an API patch and
>> must
>> +be identified as such. Moreover, any patch series containing an api patch
>> +must be identified in the patch's `--subject-prefix` as API-NEXT.
>
> +
>> +doc::
>> +Patch applies to the ODP documentation library contained in the `doc`
>> directory
>> +
>> +example::
>> +Patch applies to the ODP examples contained in the `example` directory.
>> +
>> +helper::
>> +Patch applies to the ODP helper library contained in the `helper`
>> directory.
>> +
>> +performance::
>> +Patch applies to the ODP performance test suite contained in the
>> +`test/performance` directory.
>> +
>> +linux-generic::
>> +linux-gen::
>> +l-g::
>>
>
> This last identifier cannot be as easily grepped for in a single grep of
>  "linux-gen" I dont say we have to drop it, but it feels a little less
> consistent
>

I'll drop l-g from v2


>
>
>> +Patch applies to the odp-linux reference implementation, specifically the
>> +`platform/linux-generic/` directory that contains that implementation.
>> The
>> +alternate forms `linux-gen` or `l-g` may be used to conserve line space
>> if
>> +desired.
>> +
>> +validation::
>> +Patch applies to the ODP validation test suite contained in the
>> +`test/validation` directory.
>> +
>> +Any other patch that applies to some other top-level directory or file
>> should
>> +use that directory or file name as its area.
>> +
>> +The component portion of the short log identifies the ODP component that
>> this
>> +patch applies to. A component is required unless the area is a single
>> file, in
>> +which case it may be omitted. Components should be selected from the
>> following
>> +list:
>> +
>> +buffer::
>> +Buffers and buffer processing.
>> +
>> +classification::
>> +cls::
>>
>
> class would make for a simple grep if  we accept an alternative short form
>

cls is the API abbreviation we use (e.g., odp_cls_cos_create()) so that
clearly should be
acceptable. I don't see the need to introduce class: as an abbreviation,
especially as that might be confusing to those coming from a C++
background. If we ever did go fully OO, class: would be a proper
counterpart to api: as defining a formal ODP class.


>
>
>> +Classification. `cls` may be used as an abbreviation for space if
>> desired.
>> +
>> +crypto:
>> +Crypto.
>> +
>> +pktio::
>> +Packet I/O (pktio) and its related sub-components
>> +
>> +packet::
>> +Packets and packet processing.
>> +
>> +pool::
>> +Buffer pools and related processing.
>> +
>> +queue::
>> +Queues and related processing.
>> +
>> +scheduler::
>> +sched::
>> +Scheduler and related processing.
>> +
>> +time::
>> +Time.
>> +
>> +timer::
>> +Timers and related processing.
>> +
>> +tm::
>> +Traffic Manager and related processing.
>> +
>> +Following the component the rest of the short log should describe the
>> purpose
>> +of this patch.
>> +
>> +=== Examples
>> +To illustrate, here are some examples of good and bad short logs:
>> +
>> +Good::
>> +* `api: pool: add new foo attribute to pools`
>> +* `linux-generic: pool: implement new foo attribute for pools`
>> +* `validation: pool: test new foo attribute for pools`
>> +
>> +Bad::
>> +* `fix something` (doesn't follow required format)
>> +* `api: propose a new api` (missing component)
>> +* `validation: pool: a description that rambles on and on and never gets
>> to
>> +the point`
>> +
>>  == Common Errors in Patch and Commit Messages
>>
>>  - Avoid starting the summary line with a capital letter, unless the
>> component
>> @@ -92,7 +221,7 @@ Code without a proper signoff cannot be merged into
>> the mainline.
>>
>>  - References to wikipedia are not permitted.
>>
>> -=== Documenting the code
>> +=== Documenting the Code
>>
>>  - Allow doxygen to use all its default behaviors to identify tagged
>>    information but where a doxygen tag must be specified use @
>> --
>> 2.7.4
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"
>
>
>
Bill Fischofer June 27, 2016, 4:25 p.m. UTC | #4
On Tue, Jun 14, 2016 at 7:30 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia-bell-labs.com> wrote:

> > +
> > +performance::
> > +Patch applies to the ODP performance test suite contained in the
> > +`test/performance` directory.
>
> Performance is not optimal tag since it gives an impression that some
> performance optimization was done, etc.
>
> I have used e.g. "test: l2fwd: ..."
>
>
would something like perftest: be more clear? The key is that these are not
part of the validation test suite but are useful for testing and
characterizing the performance of implementations.


>
>
> > +
> > +linux-generic::
> > +linux-gen::
> > +l-g::
> > +Patch applies to the odp-linux reference implementation, specifically
> the
> > +`platform/linux-generic/` directory that contains that implementation.
> > The
> > +alternate forms `linux-gen` or `l-g` may be used to conserve line space
> > if
> > +desired.
>
> l-g is may be too ambiguous - especially when the commit is applied to
> another implementation/repo (where l-g may have another meaning).
>

As noted, l-g will be deleted in v2.


>
>
> > +
> > +validation::
> > +Patch applies to the ODP validation test suite contained in the
> > +`test/validation` directory.
> > +
>
> I have used simply "test: sched: ...", "test: pool: ...", etc
>

Since ODP consists of three major components (API spec, implementation, and
validation suite) I think we want to clearly identify patches that are part
of the validation test suite, which is why validation: is specified here.
 test: seems too generic and could be anything.


>
> Most of the test directory content is validation tests anyway. 'Make
> check' runs also some performance tests. Examples may be distinguished with
> "example: ".
>
>
> -Petri
>
>
diff mbox

Patch

diff --git a/CONTRIBUTING b/CONTRIBUTING
index a81fd8d..b0b2781 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -11,12 +11,27 @@  in understanding the  contributing requirements for ODP
 This document is intended to guide a new application developer in understanding
 the  contributing requirements for ODP
 
+== Contributor's Agreement
+ODP is an open source project that follows the
+https://opensource.org/licenses/BSD-3-Clause[BSD 3 Clause] license. Every
+contributor to ODP must agree to comply by these licensing terms as well as
+assert that they have the right to contribute the code they submit to the
+project. No patches will be considered for acceptance without the contributor
+having first executed either an
+http://www.opendataplane.org/contributor/individual/[Individual Contributor
+License Agreement] or being a member of an orgnaization that has executed
+a http://www.opendataplane.org/contributor/corporate/[Corporate Contributor
+License Agreement].
+
+These agreements are a one-time requirement and take only a few minutes to
+perform by click-through.
+
 == New Development
 
 ODP code shall be written with the kernel coding style https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle[Kernel Coding Style]
 
 ODP code shall be documented using the doxygen style described in the
-"Documenting the code" section.
+<<Documenting the Code>> section.
 Check patch script/checkpatch.pl shall be used before submitting a patch.
 
 == ODP patch expectations as an  open source project
@@ -42,6 +57,120 @@  which can be found in https://git.kernel.org/cgit/linux/kernel/git/torvalds/linu
 
 Code without a proper signoff cannot be merged into the mainline.
 
+== Short Log and Long Log Conventions
+For ease of patch management, every submitted patch must have a short log
+entry. The short log is a one-line summary of the patch and says where it
+is intended to be applied and (briefly) its purpose. Following the short
+log, submitters are free to add additional detail in the long log. Long log
+entries are encouraged when appropriate, but there are no specific requirements
+governing their structure or contents other than that they be relvant and
+useful to potential reviewers in understanding the purpose and rationale
+for the patch.
+
+=== Short log format
+ODP requires that the short log be a single line not exceeding 80 characters,
+be composed entirely in lower case, and be structured as follows:
+
+`area: component: brief description`
+
+Note that a single colon (:) should be used to separate each section of the
+short log and be followed by a single space before beginning the next section.
+
+Area identifies the functional area that this patch addresses and in general
+says where in the directory hierarchy the patch is intended to apply. This
+should be taken from the following list:
+
+api::
+Patch adds, deletes, or changes an ODP API. Any patch that applies to the
+`include` directory or its descendants is considered an API patch and must
+be identified as such. Moreover, any patch series containing an api patch
+must be identified in the patch's `--subject-prefix` as API-NEXT.
+
+doc::
+Patch applies to the ODP documentation library contained in the `doc` directory
+
+example::
+Patch applies to the ODP examples contained in the `example` directory.
+
+helper::
+Patch applies to the ODP helper library contained in the `helper` directory.
+
+performance::
+Patch applies to the ODP performance test suite contained in the
+`test/performance` directory.
+
+linux-generic::
+linux-gen::
+l-g::
+Patch applies to the odp-linux reference implementation, specifically the
+`platform/linux-generic/` directory that contains that implementation. The
+alternate forms `linux-gen` or `l-g` may be used to conserve line space if
+desired.
+
+validation::
+Patch applies to the ODP validation test suite contained in the
+`test/validation` directory.
+
+Any other patch that applies to some other top-level directory or file should
+use that directory or file name as its area.
+
+The component portion of the short log identifies the ODP component that this
+patch applies to. A component is required unless the area is a single file, in
+which case it may be omitted. Components should be selected from the following
+list:
+
+buffer::
+Buffers and buffer processing.
+
+classification::
+cls::
+Classification. `cls` may be used as an abbreviation for space if desired.
+
+crypto:
+Crypto.
+
+pktio::
+Packet I/O (pktio) and its related sub-components
+
+packet::
+Packets and packet processing.
+
+pool::
+Buffer pools and related processing.
+
+queue::
+Queues and related processing.
+
+scheduler::
+sched::
+Scheduler and related processing.
+
+time::
+Time.
+
+timer::
+Timers and related processing.
+
+tm::
+Traffic Manager and related processing.
+
+Following the component the rest of the short log should describe the purpose
+of this patch.
+
+=== Examples
+To illustrate, here are some examples of good and bad short logs:
+
+Good::
+* `api: pool: add new foo attribute to pools`
+* `linux-generic: pool: implement new foo attribute for pools`
+* `validation: pool: test new foo attribute for pools`
+
+Bad::
+* `fix something` (doesn't follow required format)
+* `api: propose a new api` (missing component)
+* `validation: pool: a description that rambles on and on and never gets to
+the point`
+
 == Common Errors in Patch and Commit Messages
 
 - Avoid starting the summary line with a capital letter, unless the component
@@ -92,7 +221,7 @@  Code without a proper signoff cannot be merged into the mainline.
 
 - References to wikipedia are not permitted.
 
-=== Documenting the code
+=== Documenting the Code
 
 - Allow doxygen to use all its default behaviors to identify tagged
   information but where a doxygen tag must be specified use @