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

Update RFC process #3460

Merged
merged 6 commits into from
Mar 29, 2023
Merged

Update RFC process #3460

merged 6 commits into from
Mar 29, 2023

Conversation

davidmirror-ops
Copy link
Contributor

@davidmirror-ops davidmirror-ops commented Mar 13, 2023

Describe your changes

Introduces the following changes to the RFC process:

  1. Keep the Lazy Consensus approach for decision making but giving the Steering Committee a binding voting role
  2. Introduce the Final Comment Period stage
  3. Define scope of a RFC
  4. Define acceptance criteria
  5. Eliminate the need to start on Markdown and continue on PR
  6. Incline to async communication

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Signed-off-by: David Espejo <[email protected]>
@fg91 fg91 requested review from fg91 and bstadlbauer March 16, 2023 20:46
rfc/README.md Outdated

[![](https://mermaid.ink/img/eyJjb2RlIjoiZ3JhcGggVERcbiAgICBBW1N0YXJ0XSAtLT58V3JpdGUgUkZDfCBCKE9wZW4gYSBQUilcbiAgICBCIC0tPiBDe1BSIFJldmlld31cbiAgICBDIC0tPnxBcHByb3ZlZHwgRFtNZXJnZV1cbiAgICBDIC0tPnxGZWVkYmFja3wgQ1xuICAgIEMgLS0-fFJlamVjdGVkfCBGW1BSIENsb3NlZF0iLCJtZXJtYWlkIjp7InRoZW1lIjoiZGVmYXVsdCJ9LCJ1cGRhdGVFZGl0b3IiOmZhbHNlLCJhdXRvU3luYyI6dHJ1ZSwidXBkYXRlRGlhZ3JhbSI6ZmFsc2V9)](https://mermaid-js.github.io/mermaid-live-editor/edit##eyJjb2RlIjoiZ3JhcGggVERcbiAgICBBW1N0YXJ0XSAtLT58V3JpdGUgUkZDfCBCKE9wZW4gYSBQUilcbiAgICBCIC0tPiBDe1BSIFJldmlld31cbiAgICBDIC0tPnxBcHByb3ZlZHwgRFtNZXJnZV1cbiAgICBDIC0tPnxGZWVkYmFja3wgQ1xuICAgIEMgLS0-fFJlamVjdGVkfCBGW1BSIENsb3NlXSIsIm1lcm1haWQiOiJ7XG4gIFwidGhlbWVcIjogXCJkZWZhdWx0XCJcbn0iLCJ1cGRhdGVFZGl0b3IiOmZhbHNlLCJhdXRvU3luYyI6dHJ1ZSwidXBkYXRlRGlhZ3JhbSI6ZmFsc2V9)
- Gathering feedback from team or community members first, to confirm that the changes will indeed be useful
- Starting a Discussion at the [RFC Incubator](https://github.com/flyteorg/flyte/discussions/new?category=rfc-incubator) to gauge interest.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add something here (or somether below maybe) about how to transition a RFC Incubator post to the official RFC pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added some lines, wdyt?

cosmicBboy
cosmicBboy previously approved these changes Mar 23, 2023
Copy link
Contributor

@cosmicBboy cosmicBboy left a comment

Choose a reason for hiding this comment

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

This is great!

bstadlbauer
bstadlbauer previously approved these changes Mar 24, 2023
Copy link
Member

@bstadlbauer bstadlbauer left a comment

Choose a reason for hiding this comment

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

Looks good to me - thank you!

fg91
fg91 previously approved these changes Mar 24, 2023
Copy link
Member

@fg91 fg91 left a comment

Choose a reason for hiding this comment

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

Can the first version of the process diagram be removed? Only the second appears to be used in the text.

Apart from that I left minor comments and suggestions, feel free to address or ignore as you see fit.

Thanks for refining this, LGTM.

rfc/README.md Outdated Show resolved Hide resolved
rfc/README.md Show resolved Hide resolved

RFCs must be introduced as a `.md` (Markdown-format) file in a PR into the flyte repo. Details of the location to store them can be [found below](#Where-to-store-RFCs).
The Flyte repo on GitHub has an RFC folder with 3 directories:
- **Core language:** proposals to `FlyteIdl` that change the wire-format in any way are considered significant changes that require revision and approval.
Copy link
Member

@fg91 fg91 Mar 24, 2023

Choose a reason for hiding this comment

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

An example of what change to flyteidl constitutes or doesn't constitute a change of the wire-format might be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great question. @kumare3 / @cosmicBboy do you have an idea?

rfc/README.md Outdated Show resolved Hide resolved
rfc/README.md Outdated Show resolved Hide resolved
rfc/README.md Outdated
* The proposal will be discussed as much as possible in the RFC pull request directly. Any outside discussion will be summarized in the comment thread
* When deemed "ready", a maintainer or TSC member will propose a "motion for Final Comment Period (FCP)" along with a disposition of the outcome (merge, close, or postpone). This step is taken when enough discussions of the tradeoffs have taken place and the community is in a position to make a decision.
* The proposal enters FCP unless there's any objection (lazy consensus)
* The Final Comment Period will last 7 days. If there's no objection, the FCP can close early
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
* The Final Comment Period will last 7 days. If there's no objection, the FCP can close early
* The Final Comment Period will last 7 days. If there's no objection, the FCP can close early.

rfc/README.md Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Is this file still needed? Only v2 is used in the text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

Co-authored-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: David Espejo <[email protected]>
@davidmirror-ops davidmirror-ops dismissed stale reviews from fg91, bstadlbauer, and cosmicBboy via 23c1afd March 24, 2023 19:17
davidmirror-ops and others added 3 commits March 24, 2023 14:19
Co-authored-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: David Espejo <[email protected]>
@cosmicBboy cosmicBboy merged commit 42a2467 into flyteorg:master Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants