Open Stack

Hi everyone,

I believe this idea was raised briefly at a recent AC meeting, but
here's a more concrete proposal.

RFC: A streamlined process for cleanup patches
==============================================

Rationale
---------

Currently, commiting a change to Kata generally requires both a Github
issue and a linked Github pull request.  This works for bugs found "in
the wild" and for feature/enhancement requests: it makes sense to
discuss the change that needs to happen before moving on to how to
implement it.

However, this process works poorly for "cleanup" patches.  That
includes internal refactoring without changing external behaviour, and
also bugs found by inspection where it's clear that the code doesn't
behave as it should, but working backwards to find user inputs which
would trigger the bug can be quite difficult.  It's certainly possible
to write an issue for such a fix, but it often ends up just
re-iterating the same information as in the PR; the issue templates
also don't help with it since they assume the report is coming from
observation of behaviour, rather than inspection of code.

In addition cleanup patches don't work well with the procedural checks
which require each PR to have a patch labelled as fixing a Github
issue.  A well written cleanup PR will often consist of a number of
small patches which each incrementally improve the code quality, but
none of which could be said specifically to "fix" a clearly delineated
problem.  This means that someone writing a cleanup PR has to either
arbitrarily place the "fixes #" tag on the last patch (which is
misleading), or place it on all of them which could lead to
prematurely closing the issue if an incomplete series is merged my
mistake.

To reduce barriers to people addressing technical debt in Kata, I
therefore propose that we introduce a shorter process for reviewing
and merging cleanup patches.

Guidelines
----------

We don't want to stop encouraging well written reports for externally
observed bugs, or enhancement requests.  So, we need some guidelines
as to what constitutes a "cleanup" PR.

A PR which changes the code without changing any externally visible
behaviour would certainly qualify.  I think limiting to *only* that
would be too restrictive though: that would disallow PRs which
technically change behaviour but only in an obscure / unsupported /
unimportant case.  Likewise it would prevent fixing of bugs found by
inspection - we want to encourage people to fix bugs, not slow them
down with bureaucracy.

Here are some possible guidelines that admittedly leave some fuzzy
room in the middle:

Considered a cleanup patch:
  * Internal refactoring with no external behaviour change
  * Rewording comments or documentation for clarity
  * Updates to developer facing documentation to match other cleanup
    changes

NOT considered a cleanup patch:
  * Added functionality
  * Anything requiring user facing documentation to be updated to match
    code changes

Process
-------

There are several ways we could allow for a streamline cleanup
process.  Here are three options, which I'm soliciting opinions on:

  Option 1 (no issue required)

    * Cleanup PRs can be submitted with no accompanying Github issue
    * Such a PR must be tagged with a (new) 'cleanup' label
    * Scripts would be adjusted to to require a "fixes" tag on a
      cleanup labelled PR

    Advantages:
      - Minimum of overhead for small fixes
    Disadvantages:
      - Process would need to be documented elsewhere, with no ability
        for Github to guide people through it

  Option 2 (special issue template, minimal PR)

    * A new issue template for Cleanup is added alongside enhancement
      request, bug fix etc.
    * PRs attached to a cleanup issue are allowed / expected to be
      minimal, simply linking the issue with no further description
      required (individual patches in the PR should still have good
      commit messages, of course)

    Advantages:
      - Issue template can help guide people through the processz
    Disadvantages:
      - Still requires the extra step of creating both the issue and PR

  Option 3 (long-lived cleanup issues)

    * Cleanups can be attached to a specially labelled issue which
      describes not a specific problem, but rather a general area
      where cleanup / improvement might be desirable
    * PRs linked to a "cleanup" issue aren't expected to "fix" it,
      just to make some amount of improvement
    * Merging multiple PRs against the same cleanup issue would be
      normal
    * Scripts would be adjusted not to require a "fixes" tag on such a
      PR, but maybe a "improves" tag.  Processing such a PR would
      *not* close the issue

    Advantages:
      - Allows incremental improvements to be merged quickly, without
        requiring them to fully "fix" some delineated problem
    Disadvantages:
      - Unclear when the right time to (manually) close such an issue
        would be, because it's now "good enough"


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.katacontainers.io/pipermail/kata-dev/attachments/20220407/7dbd03c9/attachment.sig>