-
Notifications
You must be signed in to change notification settings - Fork 362
Add feature branch policy #2432
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
Conversation
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.
Code Review
This pull request introduces a feature branch policy and updates the documentation accordingly. The changes are a good step towards formalizing the development process. However, I've found a few inconsistencies and errors in the documentation that need to be addressed to ensure clarity and correctness. Specifically, the status of feature branches is inconsistent between files, information has been added to the wrong section in the Chinese README, and there's a grammatical error in the policy definition itself. Please see the detailed comments for suggestions.
cc @wangxiyuan @ganyi1996ppo @jianzs @ApsarasX @MengqingCao @zzzzwwjj @jgong5 Considering that this is a content involving versioning policy, it is best to get reviews from most maintainers and active contributors. My main concern is maintenance cost and branches diverge too far, therefore, I added mentor and merge plan. |
@@ -77,6 +77,7 @@ Below is maintained branches: | |||
| v0.7.1-dev | Unmaintained | Only doc fixed is allowed | | |||
| v0.7.3-dev | Maintained | CI commitment for vLLM 0.7.3 version, only bug fix is allowed and no new release tag any more. | | |||
| v0.9.1-dev | Maintained | CI commitment for vLLM 0.9.1 version | | |||
| rfc/feature-name | Maintained | [Feature branches](https://vllm-ascend.readthedocs.io/en/latest/community/versioning_policy.html#feature-branches) for collaboration | |
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 this line should be added once a new branch is really comming?
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 is just an index, I think it will not be changed in future.
Maybe we can refer to WebAssembly/proposals to tier our feature branches. |
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
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.
lgtm
Thanks for info, this PR is only for feature branch, but not for proposal workflow. But I think your link about proposals is also very useful to help us manage proprosals. |
please fix the lint error, then let's merge this |
Signed-off-by: Yikun Jiang <yikunkero@gmail.com>
Thanks all @ganyi1996ppo @MengqingCao @ApsarasX Let's first merge this to see feature branch works as our expected! If it's unexpected, please feel free to tell me, we can revert this policy. |
What this PR does / why we need it?
This patch add the feature branch policy.
After this patch: maintainers are allowed to create a feature branch. Feature branches are used for collaboration and must include an RFC link, merge plan and mentor info.
Does this PR introduce any user-facing change?
No
How was this patch tested?
CI passed