Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: add status field #177

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions 0000-template.md
Expand Up @@ -3,6 +3,7 @@
- Name: (fill in the feature name: My Feature)
- Start Date: (fill in today's date: YYYY-MM-DD)
- Author(s): (Github usernames)
- Status: Draft <!-- Acceptable values: Draft, Approved, On Hold, Superseded -->
- RFC Pull Request: (leave blank)
- CNB Pull Request: (leave blank)
- CNB Issue: (leave blank)
Expand Down
1 change: 1 addition & 0 deletions merge-rfc.sh
Expand Up @@ -146,6 +146,7 @@ if [[ "$OSTYPE" == "darwin"* ]]; then
fi
sed $SEDOPTION "s|- RFC Pull Request:.*|- RFC Pull Request: [${REPO}#${PR_NUMBER}](https://github.com/${OWNER}/${REPO}/pull/${PR_NUMBER})|" "${SOURCE_DOC}"
sed $SEDOPTION "s|- CNB Issue:.*|- CNB Issue: $ISSUES_TEXT|" "${SOURCE_DOC}"
sed $SEDOPTION "s|- State:.*|- State: **Approved**|" "${SOURCE}"

echo "> Moving ${SOURCE_DOC} to ${TARGET_DOC}..."
git mv "${SOURCE_DOC}" "${TARGET_DOC}"
Expand Down
99 changes: 99 additions & 0 deletions text/0000-add-status-field.md
@@ -0,0 +1,99 @@
# Meta
[meta]: #meta
- Name: Add Status to RFC Metadata
- Start Date: 2021-07-14
- Author(s): @aemengo, @ekcasey
- Status: Draft <!-- Acceptable values: Draft, Approved, On Hold, Superseded -->
- RFC Pull Request: (leave blank)
- CNB Pull Request: (leave blank)
- CNB Issue: (leave blank)
- Supersedes: (put "N/A" unless this replaces an existing RFC, then link to that RFC)

# Summary
[summary]: #summary

This is a proposal to add a `Status:` field to the `Meta` section of the RFC template.

# Definitions
[definitions]: #definitions

**Core team:** The party that votes on the approval of RFCs.

# Motivation
[motivation]: #motivation

This RFC formally introduces the **"On Hold"** status for an RFC, in a way that's clear to the community.

# What it is
[what-it-is]: #what-it-is

A new `Status:` field in the `Meta` section of the RFC template. See [#meta](#meta) for example.

The acceptable values are distinguished in the following ways:
- **Draft**: The Core team has not yet approved the RFC. Edits are still being made.
- **Approved**: The Core team has approved the RFC and is accepting PRs for its implementation.
- **On Hold**: The Core team has approved the RFC, but PRs are currently not being accepted for its implementation. A reason must be provided in the RFC.
- **Superseded**: The Core team has approved the RFC, but it is replaced with another RFC

The `Status:` field does not intend to reflect states that are already implied by the RFC process. Examples including: *In Review*, *Closed*, etc.

# How it Works
[how-it-works]: #how-it-works

We will add the following to the RFC `Meta` section template:

```
- Status: Draft <!-- Acceptable values: Draft, Approved, On Hold, Superseded -->
```

When an RFC is approved, the [../merge-rfc.sh](../merge-rfc.sh) script reflects the change like so:

```
- Status: Approved
```

When an RFC is put on hold (_via Pull Request_) then change is reflected like so:

```
- Status: On Hold
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Pull requests that change the status of a previously accepted RFC to "On Hold", should add a Section titled `Notes` to the top of the RFC containing an explanation for the change of status.
Example:
\```
# Notes
## On Hold
Since the approval of this RFC additional concerns X, Y, and Z have been raised. Solving problem Foo is still a priority for the project but the core team has decided to place this RFC on hold pending further discussion of alternative solutions. If you are interested in problem Foo place participate in the discussion on RFC #XXXX.
\```
If at a later date the core team or responsible sub team wishes to move the RFC from "On Hold" back to "Approved", this should be done via PR and another note should be added, describing the resolution of the discussion and the rationale for proceeding with the original RFC.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel if we want to move a RFC back off of hold there should have a superseding RFC (in most cases).

Pull requests that change the status of a previously accepted RFC to "On Hold", should add a Section titled `Notes` to the top of the RFC containing an explanation for the change of status.

Example:

```
# Notes
## On Hold
Since the approval of this RFC additional concerns X, Y, and Z have been raised. Solving problem Foo is still a priority for the project but the core team has decided to place this RFC on hold pending further discussion of alternative solutions. If you are interested in problem Foo place participate in the discussion on RFC #XXXX.
```

If at a later date the core team or responsible sub-team wishes to move the RFC from "On Hold" back to "Approved", this should be done via PR and another note should be added, describing the resolution of the discussion and the rationale for proceeding with the original RFC.

# Drawbacks
[drawbacks]: #drawbacks

* This field might get out of sync if not properly maintained.

# Alternatives
[alternatives]: #alternatives

- Communicate "On Hold" status through word of mouth
- Reject the "On Hold" status altogether

# Prior Art
[prior-art]: #prior-art

- TensorFlow has an ["Status" field](https://github.com/tensorflow/community/blob/master/rfcs/yyyymmdd-rfc-template.md)

# Unresolved Questions
[unresolved-questions]: #unresolved-questions

- Should **"Implemented"** be one of the accepted values?
- Perhaps, but then what's the process of keeping the _Implemented_ status in sync?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the goal we're trying to solve with the "Status" field? I think answering this would help answer this question.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the motivation of this RFC, it seems as though it's to clearly portray the status of any given RFC. If that's the case I would argue that knowing whether something is "Implemented" or "Implementable" is a helpful distinction.

As a prior art: Tekton TEPs use these statuses: https://github.com/tektoncd/community/tree/main/teps#teps


- How do we retroactively add a **Status:** field to prior RFCs?
- Manually?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think manually is fine.


- How do we validate the [#meta](#meta) section of prior RFCs?
- Perhaps we make a [../validate-rfcs.sh](../validate-rfcs.sh) script that lints the **#meta** section of all rfcs?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can leverage Javier's automation.