-
Notifications
You must be signed in to change notification settings - Fork 199
Use a cutoff date from BCD for Baseline high #1333
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
foolip
left a comment
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.
Self-review to make this easier to follow.
| baseline: high | ||
| baseline: low | ||
| baseline_low_date: 2022-01-06 | ||
| baseline_high_date: 2024-07-06 |
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.
Even though this date is in the past, this change is rolled back, since the latest release in BCD is Safari 17.6, released 2024-06-17.
| type LowDate = Temporal.PlainDate | string; | ||
|
|
||
| export function isFuture(date: Temporal.PlainDate): boolean { | ||
| return Temporal.PlainDate.compare(Temporal.Now.plainDateISO(), date) < 0; |
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.
Removing this line is key. Everything else was the follow-on effects.
| statuses.flatMap((s) => [...s.support.values()]), | ||
| ); | ||
| const { baseline, baseline_low_date, baseline_high_date, discouraged } = | ||
| const discouraged = statuses.some((s) => s.discouraged); |
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 pulled out discouraged, previously it was passed into keystoneDateToStatus only to be returned unchanged.
| const keystoneDate = findKeystoneDate([...s.values()]); | ||
|
|
||
| const { baseline, baseline_low_date, baseline_high_date, discouraged } = | ||
| keystoneDateToStatus(keystoneDate, f.deprecated ?? false); |
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.
Eliminating this call to keystoneDateToStatus made this change a bit simpler, and the information wasn't used anyway.
1d20ee0 to
6dbb4bd
Compare
| // findKeystoneDate is used here because it returns the latest date from an | ||
| // array of releases. The result isn't a keystone date of any feature. | ||
| const cutoffDate = findKeystoneDate(browsers(compat).map((b) => b.current())); | ||
| assert(cutoffDate !== null); | ||
|
|
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 adds a new condition to the Baseline high date: nothing can reach Baseline high until 30 months after Baseline low and at least one browser has released on or since that date.
If the aim here is to pin ourselves to BCD's state then I'd like to suggest some alternatives:
- BCD's
__meta.timestampvalue (e.g., we can never get ahead of BCD's release schedule) - The last recorded release date in BCD, even if it's a future release date (something like
browsers(compat).map((b) => b.releases.at(-1))).
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've gone with __meta.timestamp. The latest release in the releases array can be a planned release months into the future, so it's not a good approximation of now. Currently, Edge and Firefox have planned releases in September in BCD.
Fixes #1331.