Formalize review requirements for API changes

[dlorenc]

Changes to the Tekton CRDs should probably require more than one reviewer/a more formal review process than other changes. We should formalize this, document it and automate it where possible.

[dlorenc]

Ref #902 for an example of this.

[bobcatfish]

One idea (which I think was yours @dlorenc ) is to move all the _types files into a dir with its own OWNERS file (I wonder if the OWNERS file has the flexibility for us to add criteria such as "at least 3 reviews required"?)

I would like to be present in every API change review when possible.

[bobcatfish]

It's sounding (especially after the impact on projects like tektoncd/dashboard from changing step names in #818) like there is more to the API than just the CRD types so it doesn't seem like it can be as simple as moving those definitions to their own dir and changing the owners file.

@dlorenc @skaegi and I have started work on a doc about moving forward to beta which will expand our definition of what is considered part of the API. (watch this space!)

Here is a proposal for the review requirements of an API change (assuming we have agreed on a slightly more broad definition) @dlorenc @imjasonh @vdemeester (and @abayer who is likely to become an owner shortly 😉 ) please take a look:

1. API changes must be reviewed by all OWNERS: Every OWNER must explicitly /approve the PR. This doesn't mean they have to review the entire PR, just review the potential change to the API itself
2. We set the expectation that PRs that change the API will take longer to review
3. If an OWNER cannot be reached (e.g. is on vacation) the other OWNERS can decide to use their discretion and collectively sign off on that person's behalf. If the OWNER returns and disagrees with the change, and it has not been released, it should be reverted. (This means we should avoid merging API changes immediately before a release.)

Once we have the definition we can create plans around how to automate enforcement in each area, e.g. a combo of OWNERS files and some end to end tests.

[bobcatfish]

Every OWNER must explicitly /approve the PR. This doesn't mean they have to review the entire PR, just review the potential change to the API itself

Another option is like 50% + 1 but I think every owner is better if we think we can handle it.

[bobcatfish]

While reviewing yesterday it became quickly clear that:

1. Nearly every PR includes an API change of some kind!
2. Asking every OWNER to approve every one of these changes is probably too much

So here is attempt number two:

1. Backwards incompatible API changes must be approved by all OWNERS: Every OWNER must explicitly /approve the PR. This doesn't mean they have to review the entire PR, just review the potential change to the API itself. This means the PR may stay open for a while, e.g. if an OWNER is unavailable for several weeks.
2. Additive API changes must be approved by at least two OWNERs

[bobcatfish]

We have also been working on a doc about how we can get to Beta for Pipelines, we can work on this as part of this issue.

[bobcatfish]

Notes from our recent governance meeting, which I will update our docs with:

1. Backwards incompatible (i.e. making the change in a backwards compatible manner)= tag all OWNERS, then 50% + 1 minimum approved
2. Additive changes = Two OWNERS should approve