-
Notifications
You must be signed in to change notification settings - Fork 284
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
Remove feature flags from accepted RFCs #872
Conversation
9f2f779
to
4e89b0e
Compare
4e89b0e
to
e2c8e58
Compare
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 all looks good. The only change is as we discussed: let's add the features back in as no-ops to avoid an unnecessary breaking change, and we'll remove them for good at 1.0.
@dherman After some testing, I think our best option to give a deprecation warning is to go through and on each public export that uses the feature flag, replace it with: #[cfg_attr(feature = "feature-flag", deprecated(message = "`feature-flag` is enabled by default; remove the flag"))] |
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!
c58c63d
to
0e0c126
Compare
1f0fcb7
to
018616a
Compare
Background
Neon uses feature flags to provide early access to features that may change while the RFC is still open. However, after accepting an RFC, the feature flag should be removed and the feature provided by default.
Overview
This PR removes several feature flags (
try-catch-api
,channel-api
,event-queue-api
,promise-api
, andtask-api
). These features all have accepted RFCs.Additionally, feature flags that are only used for the
legacy
backend are commented as deprecated. This will serve as a note to remove when removing thelegacy
backend.Alternative
This PR completely removes the feature flags. This is a breaking change that will require users to remove the feature flag when upgrading.
Alternatively, Neon could only remove usages of the feature flag, but keep the flag in
Cargo.toml
for backwards compatibility, deferring the breaking change to a later release.Follow-up
The
enable-static-tests
feature flag should also be removed. #871 was filed as a follow-up for this effort.