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

Remove feature flags from accepted RFCs #872

Merged
merged 8 commits into from
Mar 23, 2022
Merged

Conversation

kjvalencik
Copy link
Member

@kjvalencik kjvalencik commented Mar 7, 2022

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, and task-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 the legacy 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.

Copy link
Collaborator

@dherman dherman left a 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.

@kjvalencik
Copy link
Member Author

@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"))]

@kjvalencik kjvalencik requested a review from dherman March 22, 2022 20:26
Copy link
Collaborator

@dherman dherman left a comment

Choose a reason for hiding this comment

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

LGTM!

@kjvalencik kjvalencik merged commit 5cf493e into main Mar 23, 2022
@kjvalencik kjvalencik deleted the kv/remove-feature-flags branch March 23, 2022 20:07
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.

2 participants