Unstable Features for faster iterations #52
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # Feature Name: `unstable_features` | ||
|
|
||
| ## Summary | ||
|
|
||
| Experimental new features are merged into main behind a feature gate before stabilization. | ||
|
|
||
| ## Motivation | ||
mockersf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Some features would benefit from iterative work in Bevy, but that may be hard to do when a PRs need a high level of quality to be merged in. | ||
|
|
||
| Merging new features behind a feature gate could improve this situation. New features could start getting user feedback, helping reach a higher quality before stabilization. | ||
|
|
||
| This would also improve the contributor experience for PRs that stay open for some time and ends up conflicting with main. | ||
|
|
||
| In this RFC, "feature" means the new feature or change under development, and "feature gate" the conditional compilation flag. "unstable" means a feature that is subject to change, not something that will break Bevy. | ||
|
|
||
| ## User-facing explanation | ||
|
|
||
| So you want to contribute a new feature to Bevy! You have a few paths available to you, depending on the scope of the feature: | ||
| - A small feature with a clear implementation? Go ahead and open that PR! | ||
| - A large feature with an open design space that could use some discussion about how to implement it? A RFC would be good. | ||
| - Something in between that would benefit from iterative work? Here come the unstable feature workflow for you! | ||
|
There was a problem hiding this comment. I think large RFC'ed changes should also be subject to this workflow too. It'd also help isolate the effects of implementing individual parts of the larger design before it becomes stabilized. There was a problem hiding this comment. Yes I agree, but in those case I think the workflow should be RFC first, then unstable feature once the RFC is more or less agreed on. I tried to mention that in the "implementation" part, would that work for you? There was a problem hiding this comment. I see it now, but it's not particularly focused upon. Could you expand on each potential workflow down below? |
||
|
|
||
| You will need to open a tracking issue to explain the new feature you want to add. If it looks good, you'll be ready to open that PR with a feature gate for your changes! | ||
|
|
||
| ## Implementation strategy | ||
|
There was a problem hiding this comment. I think a convention for feature flags naming should be kept so that users can quickly discern which features are unstable, and we should have some log or central location to track these features. Also unfortunately feature flag doc generation are currently bound to nightly. Not sure how to get around this before that feature gets stabilized. There was a problem hiding this comment. Agreed on the convention. I think it's good that the doc won't be built for those unstable features by default. If someone wants it, the doc can be built on demand with the feature gate enabled. There was a problem hiding this comment. On one hand, I'm inclined to agree that we shouldn't be putting this forth front and center, but that also hides these features from those who might both benefit from them in the short term and be a good testbed for stabilization. By showing them and explicitly labeling APIs as "unstable, expect bugs", I think that should deter any user expecting stability. The same goes for nightly-bound unstable features in Rust: it garners both testers and active feedback. There was a problem hiding this comment. I think a GitHub project showing tracking the stabilisation issues should be good enough for discovery, what do you think? There was a problem hiding this comment.
For comparison, Rust has the RFC book. A GitHub Project might be good enough as a start, but IMO if we want this to be a publicly recognizable format, we need more than that in the long run. There was a problem hiding this comment. Having something like the RFC book requires quite a bit more work... It is needed in Rust where they need to keep track of everything back to the pre 1.0 time for people still using an old version, they have feature opened 7 years ago not yet closed, and they track a lot of them. I think filtering issues on the label, and presenting them in a project would be enough for some time There was a problem hiding this comment.
This is hard to discover for Bevy users which are not contributors. There was a problem hiding this comment. One edge case I would like to see explored is changes where a feature is controversial / not-yet-ready, but it needs small engine tweaks to support it. Presumably, we would have PRs that submit a mix of feature-gated and ungated changes. IMO this is better, because it keeps the feature flag localized, rather than creating branching throughout the entire code base. There was a problem hiding this comment. If the small engine tweaks are non controversial, they should be split in another PR, that's already what we try to promote There was a problem hiding this comment. The small engine tweaks cannot always be purely additive, I can imagine sometimes they're breaking changes. |
||
|
|
||
| The new workflow for this kind of feature would be: | ||
| - Submit an issue explaining the feature you want to add under a feature gate. This will be the tracking issue for stabilization once approved. This issue gets a new label `S-Stabilization`. | ||
| - If approved, submit a PR with the new feature and the feature gate | ||
| - This PR can be merged by someone with the merge rights on the related part of Bevy as soon as it's `S-Ready-For-Final-Review`, without an approval by the main Bevy maintainers. | ||
|
There was a problem hiding this comment. We should further expand this. Currently there are three people with merge rights, and only one of them is unscoped. This, at best, seems to still have a heavy bottleneck here. We already have engine teams for specific focus areas. Would it be possible to set it up such that areas of the engine could have these unstable features merged with significant focus area engine team buy-in? The only other way is to expand the number of org members with scoped merge rights, and that seems to be the slowest area of growth for the organization as a whole. There was a problem hiding this comment. I think that is out of scope of this RFC. I wrote it mentioning roles, who has those roles is not the subject. What you propose would still work in the context of this RFC, as it can be read as "giving the focus area teams the scope to merge approved unstable features PRs". There was a problem hiding this comment. Can we mark this as resolved now that all maintainers have merge rights? |
||
| - Subsequent PRs on the feature must be linked to the tracking issue and follow the same process. | ||
| - Once the feature is finished, enter the stabilization period. Open a PR that will remove the feature gate. This PR needs to be approved by the main Bevy maintainers. | ||
|
|
||
| The feature gate should follow the following convention for its name: `unstable-<#tracking-issue-number>-<feature-name>`. | ||
|
|
||
| An existing PR can be retroffited in this process on the suggestion of Bevy community members, and if the PR author agrees. | ||
|
|
||
| The RFCs and the unstable features are closely related. An RFC can be implemented as an unstable feature to help resolve issues that need more experimentation, or split a large implementation. An unstable feature could require an RFC to reach stabilization. For example: | ||
| - RFC to Unstable Feature | ||
| - Open a RFC, reaching a design | ||
| - Implement part of the RFC as an unstable feature | ||
| - Feedback into the RFC with the gained knowledge of implementation and usage | ||
| - Unstable Feature to RFC | ||
| - Open an tracking issue, start implementing it | ||
| - Part of the unstable feature is larger than expected with several design possibilities | ||
| - Open a RFC, reaching a decision on the design | ||
| - Implement the RFC for the unstable feature | ||
|
|
||
|
|
||
| Each PR will still need to be approved by Bevy community members, and would still need to have the same level of quality as other PRs merged. | ||
|
There was a problem hiding this comment. Review this inline with more recent rules about maintainers and reviewers? The rules is 2 community reviews I think. |
||
|
|
||
| The PRs for the initial feature, various changes during its finalization and for the stabilization can be opened by different persons. | ||
|
|
||
| The feature gate should not be exposed on the main `bevy` crate, but only on the subcrates where it is relevant. It can be enabled as a user by adding dependencies directly on the subcrates and enabling the feature. The tracking issue should list the subcrates impacted. | ||
mockersf marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. What is the rationale behind this choice? Piping unstable features up to the main crate would both be more convenient for consumers, and would help document the "scope" of the feature. It does add a bit more complexity, but it seems "warranted". There was a problem hiding this comment. I would prefer to not have different classes of features on the main crate. Currently we have a few to enable functionalities, feature gates for unstable features would be a different type. Moving the feature up to |
||
|
|
||
| For organization and discovery, a GitHub project can be used to track the stabilization issues. | ||
|
|
||
|
There was a problem hiding this comment. I think we should discuss adding an "easily reversible" constraint for unstable features. I can see people wanting to make large, controversial architectural changes to code "outside" of the feature flag (aka in "stable bevy code"). If we decide an unstable feature is "undesirable", removing code inside feature flag blocks should be enough to undo all of the changes. Phrased another way: people shouldn't be able to entangle "unstable" code with "stable" code. All unstable code must be contained in "unstable feature flag blocks". This constraint would make it harder to implement certain classes of features, but it would help maintain the integrity of the code base. |
||
| While it should be avoided, if another PR breaks an unstable feature, it is not the PR author responsability to fix it. They may do it if they want, otherwise it must be reported on the unstable feature tracking issue. | ||
|
|
||
|
There was a problem hiding this comment. Not something to add to this RFC, but it would be useful to come up with some "current" PRs that would benefit from being unstable features. There was a problem hiding this comment. The PR that motivated me to write that RFC is bevyengine/bevy#3884 As you said yourself
This RFC generalises that possibility: to have an advanced feature that we want to test, and let the interested people manage until it's ready for common use |
||
| ## Drawbacks | ||
mockersf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| This will increase complexity in Bevy code. Increased complexity can be controlled by limiting the number of unstable features available at a given time, or their scope. | ||
|
|
||
| It will also increase complexity of a contribution that would like to use this process, but it is opt-in and not mandatory. Think of it as a fast track for unstable features. | ||
|
|
||
| Stabilizing a feature could be harder, as it would mean a review across several PRs, and may not easily work with GitHub review UI. But the stabilization review should be easier than a large PR as there would be actual usage feedback. | ||
|
|
||
| An unstable feature accepted in Bevy may stop working due to the merge of another PR. This is the same burden as keeping a PR up to date with Bevy while it's waiting to be merged, but more visible. On the positive side, it would be easier for someone else to jump in and fix it. | ||
|
|
||
| ## Rationale and alternatives | ||
|
|
||
| - Alternatives: | ||
| - Continue as it is now, with semi large PRs that bitrot and RFCs that don't often move forward | ||
mockersf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - Grant scoped merge rights to teams / team leads for more areas of the Bevy engine, and tell users to deal with the instability | ||
| - Work harder to implement a new feature as a third party crate (this is not always possible) | ||
| - Fork Bevy to merge unapproved PRs and check how they work / interact | ||
|
|
||
| ## Prior art | ||
mockersf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| - [The Rust process](https://rustc-dev-guide.rust-lang.org/implementing_new_features.html) | ||
| - [Discussion about slowness of RFCs](https://www.reddit.com/r/rust/comments/sy65f7/we_need_to_talk_about_rfcs/) | ||
|
|
||
| ## Unresolved questions | ||
|
|
||
| - Do we want to limit the number of feature gates in Bevy? | ||
| - this could help control the increased complexity in Bevy | ||
| - Do we want to limit the maximum age of a unstabilized feature? | ||
|
There was a problem hiding this comment. With the Bevy train release schedule, it'd be nice to have a minimum time for this too, to allow for folks using the released versions of Bevy to have to test out and file bugs. This could be mitigated by telling users to use the As an upper bound, if a unstable feature doesn't get stabilized within two train releases, even with external users testing it out, I'd say we either need to reconsider the design of it as a full RFC or just scrap the idea entirely. There was a problem hiding this comment. With Rust as an example where stabilising a feature can take a few years, I think a good scope would be at least one version, at most 4 (so between 3 months and a year) There was a problem hiding this comment. I like both the upper and lower bounds here. The upper bound in particular results in a nice forcing function to actually ship things or cut our losses. |
||
| - this could help control the increased complexity in Bevy, and requires being ready to cut a feature off if it's not getting the use / stabilization 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.
I would propose we call these experimental features something other than "unstable", since a "stable feature" implies that we won't change the API of that feature going forwards; we don't make that guarantee about basically anything in Bevy at this point.
Rust has this to say about their own "unstable/stable" feature process:
Even a "stable feature" under the text of this RFC, as with almost all features in Bevy right now, is subject to change and might be completely rewritten or removed.
Plenty of good alternatives:
experimental,prototype,canary,proto,future, et c.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.
prototype is my favorite of those. Definitely like it better than unstable in the context of Bevy.
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 like it too - especially given the parallel to
bevy_prototype_<name>community crates which, from other discussion here, it sounds like will still be encouraged where possible.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 a valid point and I’d like to cast my vote on
experimental. I think the others are a bit more vague/formal/rigid whereasexperimentalfeels more approachable. To me it says, "this is an experiment 🧪 to gain feedback and toy around with implementations to see if we (as a community) want to pursue this feature or a feature like this."Edit: I also just realized the pun with
canary.... Nicely done 😂