[ovs-dev] submitting-patches: Update test and documentation recommendations.

Rationale:

- "make distcheck" is not as necessary anymore because we have a build-time
  check that fails if files in the repository are not distributed.

- xenserver has not been important for years, so remove the specific
  callout.

- We already have an informal custom of adding tests for new feaures and
  bug fixes, so codify it.

- Add note about updating NEWS.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 .../internals/contributing/submitting-patches.rst  | 25 +++++++++++-----------
 1 file changed, 12 insertions(+), 13 deletions(-)

On Thu, May 18, 2017 at 05:32:59AM -0700, Ben Pfaff wrote:
> Rationale:
> 
> - "make distcheck" is not as necessary anymore because we have a build-time
>   check that fails if files in the repository are not distributed.
> 
> - xenserver has not been important for years, so remove the specific
>   callout.
> 
> - We already have an informal custom of adding tests for new feaures and
>   bug fixes, so codify it.
> 
> - Add note about updating NEWS.
> 
> Signed-off-by: Ben Pfaff <blp@ovn.org>

I'd appreciate review of this documentation-only patch.

On 5/18/17, 5:32 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ben Pfaff" <ovs-dev-bounces@openvswitch.org on behalf of blp@ovn.org> wrote:

    Rationale:
    
    - "make distcheck" is not as necessary anymore because we have a build-time
      check that fails if files in the repository are not distributed.
    
    - xenserver has not been important for years, so remove the specific
      callout.
    
    - We already have an informal custom of adding tests for new feaures and
      bug fixes, so codify it.
    
    - Add note about updating NEWS.
    
    Signed-off-by: Ben Pfaff <blp@ovn.org>

    ---
     .../internals/contributing/submitting-patches.rst  | 25 +++++++++++-----------
     1 file changed, 12 insertions(+), 13 deletions(-)
    
    diff --git a/Documentation/internals/contributing/submitting-patches.rst b/Documentation/internals/contributing/submitting-patches.rst
    index edbc1aa8d9ff..4f7ca0304034 100644
    --- a/Documentation/internals/contributing/submitting-patches.rst
    +++ b/Documentation/internals/contributing/submitting-patches.rst
    @@ -44,29 +44,28 @@ particular:
     - A patch should make one logical change.  Don't make multiple, logically
       unconnected changes to disparate subsystems in a single patch.
     
    -- A patch that adds or removes user-visible features should also update the
    -  appropriate user documentation or manpages.  Check "Feature Deprecation
    -  Guidelines" section in this document if you intend to remove user-visible
    -  feature.
    +- A patch that adds or removes user-visible features should also
    +  update the appropriate user documentation or manpages.  Consider
    +  adding an item to NEWS for nontrivial changes.  Check "Feature
    +  Deprecation Guidelines" section in this document if you intend to
    +  remove user-visible feature.
     
     Testing is also important:
     
    -- A patch that modifies existing code should be tested with ``make
    -  check`` before submission.  Refer to the `install guide`, under "Self-Tests",
    -  for more information.
    +- Test a patch that modifies existing code with ``make check`` before
    +  submission.  Refer to the `install guide`, under "Self-Tests", for
    +  more information.

I think “Testing” is under Documentation and the label is “Unit Tests”
I cannot find the title “Self-Tests”, but admittedly, I have little patience.

Maybe checking the system tests (kernel and userspace) should be strongly suggested, 
if not an expectation.

     
    -- A patch that adds or deletes files should also be tested with ``make
    +- Consider testing a patch that adds or deletes files with ``make
       distcheck`` before submission.
     
     - A patch that modifies Linux kernel code should be at least build-tested on
       various Linux kernel versions before submission.  I suggest versions 3.10 and
       whatever the current latest release version is at the time.
     
    -- A patch that modifies the ofproto or vswitchd code should be tested in at
    -  least simple cases before submission.
    -
    -- A patch that modifies xenserver code should be tested on XenServer before
    -  submission.
    +- A patch that adds a new feature should add appropriate tests for the
    +  feature.  A bug fix patch should preferably add a test that would
    +  fail if the bug recurs.
     
     If you are using GitHub, then you may utilize the travis-ci.org CI build system
     by linking your GitHub repository to it. This will run some of the above tests
    -- 
    2.10.2
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=vV-qzeme225zuTayBoQVI5Qb9Cx0DZaeSRShNoyY1W8&s=6Vk6zDH4PRJGGw3UKXtOVXyIlJ3grOlwabbxT-K4wFE&e=

On Fri, Jul 07, 2017 at 12:18:43AM +0000, Darrell Ball wrote:
> 
> 
> On 5/18/17, 5:32 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ben Pfaff" <ovs-dev-bounces@openvswitch.org on behalf of blp@ovn.org> wrote:
> 
>     Rationale:
>     
>     - "make distcheck" is not as necessary anymore because we have a build-time
>       check that fails if files in the repository are not distributed.
>     
>     - xenserver has not been important for years, so remove the specific
>       callout.
>     
>     - We already have an informal custom of adding tests for new feaures and
>       bug fixes, so codify it.
>     
>     - Add note about updating NEWS.
>     
>     Signed-off-by: Ben Pfaff <blp@ovn.org>
>     ---
>      .../internals/contributing/submitting-patches.rst  | 25 +++++++++++-----------
>      1 file changed, 12 insertions(+), 13 deletions(-)
>     
>     diff --git a/Documentation/internals/contributing/submitting-patches.rst b/Documentation/internals/contributing/submitting-patches.rst
>     index edbc1aa8d9ff..4f7ca0304034 100644
>     --- a/Documentation/internals/contributing/submitting-patches.rst
>     +++ b/Documentation/internals/contributing/submitting-patches.rst
>     @@ -44,29 +44,28 @@ particular:
>      - A patch should make one logical change.  Don't make multiple, logically
>        unconnected changes to disparate subsystems in a single patch.
>      
>     -- A patch that adds or removes user-visible features should also update the
>     -  appropriate user documentation or manpages.  Check "Feature Deprecation
>     -  Guidelines" section in this document if you intend to remove user-visible
>     -  feature.
>     +- A patch that adds or removes user-visible features should also
>     +  update the appropriate user documentation or manpages.  Consider
>     +  adding an item to NEWS for nontrivial changes.  Check "Feature
>     +  Deprecation Guidelines" section in this document if you intend to
>     +  remove user-visible feature.
>      
>      Testing is also important:
>      
>     -- A patch that modifies existing code should be tested with ``make
>     -  check`` before submission.  Refer to the `install guide`, under "Self-Tests",
>     -  for more information.
>     +- Test a patch that modifies existing code with ``make check`` before
>     +  submission.  Refer to the `install guide`, under "Self-Tests", for
>     +  more information.
> 
> I think “Testing” is under Documentation and the label is “Unit Tests”
> I cannot find the title “Self-Tests”, but admittedly, I have little patience.
> 
> Maybe checking the system tests (kernel and userspace) should be strongly suggested, 
> if not an expectation.

Thanks, I fixed that stuff up and applied this to master.