-
Notifications
You must be signed in to change notification settings - Fork 836
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
Add AFP features and update documentation #4061
base: main
Are you sure you want to change the base?
Conversation
…documentation for AFP guidelines - Introduced a new GitHub issue template for feature proposals - Updated site documentation to include detailed instructions for the Agones Feature Proposal (AFP) process - Included metadata schema and workflow explanations in the documentation - Added references to the AFP template and review process
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Build Succeeded 🥳 Build Id: c581a967-0dc9-4082-96fa-7838dd79a85c The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Build Succeeded 🥳 Build Id: 8d6f39d9-0a4f-4a38-9166-e9d3ebb348e7 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Build Succeeded 🥳 Build Id: 96aa270d-e67e-4e91-9e18-5cfd3d2de7b4 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Build Succeeded 🥳 Build Id: 61d23164-db1d-4163-b70e-9122ea736451 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you were following exactly how KEP is setup, which I don't think is necessary since it's too comprehensive for us. For our case, we just need to document the process of AFP, which is majorly the below part in the description of #3882 (which majorly including creates a PR and an issue), document the structure of the AFP PR, and probably also provide an example AFP PR template in the agones/docs/proposal
folder. The structure of the AFP PR can be similar as the top level structure of this design doc: #2716
The process of AFP, quote from #3882 :
"When someone wants to write a design, start with an issue outlining the problem. After that, they have two options:
For smaller changes, writing it in the issue is still fine.
For larger changes, or when asked by a maintainer because the conversation is getting unwieldy, they submit an AFP PR
An AFP PR is as simple as a PR that adds a file in agones/docs/features. The PR should be titled AFP-00x and the file named 00x-feature-name.md.
Everyone can then participate on the PR, selecting text they want to comment on and having conversations there.
When we reach lazy consensus, a maintainer can approve and submit.
Every AFP OR should be tied back to an issue"
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Build Succeeded 🥳 Build Id: f8bd7609-9636-4d99-84c7-9186fbba2821 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Build Failed 😭 Build Id: a5c91ba0-70b6-44b0-aa87-8c66440de017 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Build Failed 😭 Build Id: f6a4b766-fcf1-46a8-b6cc-1ecb3926b163 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 🥳 Build Id: c3fa95e9-7bf5-45ac-9414-2c206eb50d64 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
@@ -0,0 +1,34 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok to just keep the existing feature_request.md
file. In that file, we can add the following entry:
"
Link to the Agones Feature Proposal (if any)
The link to the AFP PR.
"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my above comments still makes sense. The idea is for smaller changes we still want to give contributer a way to just state their design in the feature request issue. For large larger changes, they can submit an AFP PR and link to the feature request issue.
Without a standardized mechanism for describing important features, our | ||
talented technical writers and product managers struggle to weave a coherent | ||
narrative explaining why a particular release is important. Additionally, adopters | ||
of critical infrastructure such as Agones need a forward-looking roadmap in | ||
order to plan their adoption strategies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve removed the mentioned section as suggested
|
||
AFPs are checked into the Features repo under the `/docs/proposals` directory. | ||
|
||
New AFPs can be checked in with a file name in the form of `draft-YYYYMMDD-my-title.md`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep the file name simple as 00x-feature-name
, where 00x is a an incremental AFP number. And the PR should be titled with AFP-00x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No other changes should be put in that PR so that it can be approved quickly and minimize merge conflicts. | ||
The AFP number can also be done as part of the initial submission if the PR is likely to be uncontested and merged quickly. | ||
|
||
### AFP Editor Role |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it simple and not have this role. Author, reviewer, approver (probabaly same as reviewer) should be enough, and reviewer, approver can take the responsibility of the editor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve changed the content mentioned as suggested
[Kubernetes AFP process]: https://github.com/kubernetes/enhancements/tree/master/afps | ||
[Rust RFC process]: https://github.com/rust-lang/rfcs | ||
|
||
## Drawbacks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems not related to our case, can remove this part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
@@ -0,0 +1,64 @@ | |||
# Agones Feature Proposals (AFPs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this README serves similar purpose of the files in that 0000-afp-process
folder? Seems like they both give an introduction of what AFP's process looks like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/proposals/README.md
file has the Quick Start and FAQ sections.
0000-afp-process
folder contains all the details about AFP features.
|
||
### How to Submit an AFP | ||
|
||
To submit a new AFP, follow the instructions provided in the official AFP template. This template outlines the required sections and metadata, such as the title, authors, reviewers, and approval statuses. AFPs are stored in the project's **docs/proposals** directory, with filenames formatted as `NNNN-my-title.md`. The document undergoes iterative updates, with changes tracked via version control. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a link to the template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
links has been added now!
- **[AFP Template](#)**: A detailed template for creating an AFP. | ||
- **[AFP Metadata](#)**: Information on the metadata required for each AFP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two links do not work as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Links has been updated now
### Examples of AFPs | ||
|
||
Here are some example AFPs that have been proposed or implemented in Agones: | ||
|
||
- **[123-AFP: Example Feature](#)**: Description of a feature proposal. | ||
- **[124-AFP: Example Architecture Change](#)**: Description of a major architectural change proposal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have any example yet, no value to provide these fake links.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I go ahead and disable this session for now?
Build Succeeded 🥳 Build Id: e4f888a3-c8f1-4ac4-93dd-4aa705027602 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Failed 😭 Build Id: 0041b5de-f08f-4205-b8d0-a0b16e01c785 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😭 Build Id: 11bc9dee-8b38-4358-8722-a789050d6e56 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
@@ -0,0 +1,34 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my above comments still makes sense. The idea is for smaller changes we still want to give contributer a way to just state their design in the feature request issue. For large larger changes, they can submit an AFP PR and link to the feature request issue.
@@ -0,0 +1,85 @@ | |||
--- | |||
title: "Agones Feature Proposal (AFP)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we retile this to Feature Request, and in the content, state it clearly that for small changes, writing it in the issue is still fine, and for larger changes, or when asked by a maintainer because the conversation is getting unwieldy, they should submit an AFP PR. And then describe what AFP is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can i restore the current feature request template?
https://github.com/googleforgames/agones/blob/main/.github/ISSUE_TEMPLATE/feature_request.md
do you want to add any additional points here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think it's ok to just keep the existing feature_request.md file. In that file, we can add the following entry:
"
Link to the Agones Feature Proposal (if any)
The link to the AFP PR.
"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -0,0 +1,271 @@ | |||
# Agones Feature Proposal Process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you feel this whole file is duplicated with the descriptions in the template README? It's long and I feel distracting and a little confusing. Can we just remove the 0000-afp-process
folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's consider this as the First AFP 0000.
The template README includes the basic workflow and FAQs, but AFP 0000 includes some important extra information, such as:
Would it be better to combine all this information into one README file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to combine them together into the template README file. And then we can start from there to remove duplicated informations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the content based on your suggestions.
Build Failed 😭 Build Id: f1bab576-57bb-4a38-b19b-ff9982897d42 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
/gcbrun |
Build Failed 😭 Build Id: 56186259-5f8a-40e9-b592-52b927496495 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😭 Build Id: 485104be-bba2-4846-b2e1-cdd585eb7309 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
/gcbrun |
Build Succeeded 🥳 Build Id: b140e069-696a-48b9-bca0-081a363aace8 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
|
||
The AFP process is still evolving! | ||
If something is missing or not answered here feel free to reach out to [Slack](https://join.slack.com/t/agones/shared_invite/zt-2mg1j7ddw-0QYA9IAvFFRKw51ZBK6mkQ). | ||
If you want to propose a change to the AFP process you can open a PR on [AFP-NNNN](https://github.com/googleforgames/agones/issues/new/choose) with your proposal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
- **[AFP Template](https://github.com/googleforgames/agones/blob/main/docs/proposals/NNNN-afp-template/README.md)**: A detailed template for creating an AFP. | ||
- **[AFP Metadata](https://github.com/googleforgames/agones/blob/main/docs/proposals/README.md#afp-metadata)**: Information on the metadata required for each AFP. | ||
- **[How to Propose a Feature](https://github.com/googleforgames/agones/issues/new/choose)**: Step-by-step instructions on how to submit a new AFP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What type of PR is this?
/kind breaking
What this PR does / Why we need it:
Implement new AFP features, add AFP issue template, and enhance site documentation for AFP guidelines
Which issue(s) this PR fixes:
Works on #3882
Special notes for your reviewer: