Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Remove flag data for default-enabled features #2297

Merged
merged 2 commits into from
Sep 14, 2022

Conversation

jugglinmike
Copy link
Contributor

A recent change in the BCD project policy stipulates that information
about feature flags should be removed whenever a feature becomes
supported by default [1]. Modify the update-bcd script to honor the
new policy whenever new test results trigger that condition.

Include a test for the case where positive test results are added for a
feature that is already supported via a prefix. This patch intentionally
doesn't change that behavior because prefixes "are part of the web
platform that web developers have to deal with." [2] The test is to
prevent regressions and emphasize the intention.

[1] mdn/browser-compat-data#16637
[2] https://matrix.to/#/!cGYYDEJwjktUBMSTQT:mozilla.org/$JTWeMpzNMULBTZcx-pWFnUNX7DIzE5xPySd70Kpr-MU?via=mozilla.org&via=matrix.org&via=igalia.com),

A recent change in the BCD project policy stipulates that information
about feature flags should be removed whenever a feature becomes
supported by default [1]. Modify the `update-bcd` script to honor the
new policy whenever new test results trigger that condition.

Include a test for the case where positive test results are added for a
feature that is already supported via a prefix. This patch intentionally
doesn't change that behavior because prefixes "are part of the web
platform that web developers have to deal with." [2] The test is to
prevent regressions and emphasize the intention.

[1] mdn/browser-compat-data#16637
[2] https://matrix.to/#/!cGYYDEJwjktUBMSTQT:mozilla.org/$JTWeMpzNMULBTZcx-pWFnUNX7DIzE5xPySd70Kpr-MU?via=mozilla.org&via=matrix.org&via=igalia.com),
@@ -648,6 +710,8 @@ describe('BCD updater', () => {
chrome: [{version_added: '0> ≤83', version_removed: '85'}]
},
'api.ExperimentalInterface': {chrome: [{version_added: '0> ≤83'}]},
'api.UnflaggedInterface': {chrome: [{version_added: '0> ≤84'}]},
Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, this 0> is kind of weird :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

0> should not be appearing in output values, hmm...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GitHub's "review" interface is obscuring the context a bit, so it may not be clear that this particular test is for the inferSupportStatements function. That may include 0> in its return value, but it may not be what you mean by "output," at least not from the perspective of someone using the update-bcd script.

The update function, on the other hand, contains a handful of statements intended to remove 0>. This patch exercises this replacement; disabling that code causes the test for the update function to fail (specifically for the UnflaggedInterface and UnprefixedInterface added here).

@foolip
Copy link
Owner

foolip commented Sep 1, 2022

Hmm, tests are actually failing?

@jugglinmike
Copy link
Contributor Author

My bad--didn't realize the test fixture was used in other contexts. It's fixed, now!

@foolip foolip merged commit d5aaa43 into foolip:main Sep 14, 2022
jugglinmike added a commit to bocoup/mdn-bcd-collector-bc-legacy that referenced this pull request Sep 27, 2022
jugglinmike added a commit to bocoup/browser-compat-data that referenced this pull request Sep 28, 2022
These changes were generated procedurally by the mdn-bcd-collector
project using data collected from Chrome for Android and following three
corrections to its interpretation of the "mirror" support value:

foolip/mdn-bcd-collector#2326
foolip/mdn-bcd-collector#2297
foolip/mdn-bcd-collector#2280
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants