Skip to content

Add new impliedByAfterDate annotation to compatibility-date #2332

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

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jun 26, 2024

The new annotation allows us to specify that a compatibility flag is implied if the named other flag is enabled, but only at the specified compatibility date or later. If the flag is marked experimental and experimental flags are not allowed, or if the flag is explicitly turned off, the annotation will not apply.

This will be used, for instance, to specify that nodejs_compat_v2 is implied on by nodejs_compat after a given date.

@jasnell jasnell requested review from a team as code owners June 26, 2024 00:21
@jasnell jasnell force-pushed the jsnell/add-implied-by-after-date-annotation branch from 39075db to 5bc0405 Compare June 26, 2024 00:27
@irvinebroque
Copy link
Collaborator

To confirm, what this lets us then provide is:

  • If you are using nodejs_compat today, and have a compat date of 06-25-24 — nothing changes
  • If you are then change your compat date to <future date> — we will (behind the scenes) use nodejs_compat_v2 instead

...allowing us to maintain existing behavior, and provide an explicit opt-in (via a compatibility date)

@jasnell
Copy link
Member Author

jasnell commented Jun 26, 2024

Yes, that's exactly it @irvinebroque

Copy link
Member

@kentonv kentonv left a comment

Choose a reason for hiding this comment

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

Code looks good but can we add a test to compatibility-date-test.c++? Can base the test around the nodejs_compat_v2 flag specifically.

@jasnell jasnell force-pushed the jsnell/add-implied-by-after-date-annotation branch from 5bc0405 to 8634bbe Compare June 26, 2024 16:45
@jasnell jasnell requested a review from kentonv June 26, 2024 16:46
@jasnell
Copy link
Member Author

jasnell commented Jun 26, 2024

Test added. experimental check removed.

Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

I can't review the c++ code, but the tests look exactly like what I would expect. I just have some minor product comments.

@jasnell jasnell requested a review from IgorMinar June 28, 2024 01:30
The new annotation allows us to specify that a compatibility flag is
implied if the named other flag is enabled, but only at the specified
compatibility date or later.

This will be used, for instance, to specify that `nodejs_compat_v2`
is implied on by `nodejs_compat` after a given date.
@jasnell jasnell force-pushed the jsnell/add-implied-by-after-date-annotation branch from 754a36e to 27be8f3 Compare June 28, 2024 19:22
@jasnell jasnell merged commit 27be8f3 into main Jun 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants