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

Fix mirroring from upstream to downstream with ranges #24155

Merged
merged 14 commits into from
Dec 3, 2024

Conversation

queengooborg
Copy link
Contributor

@queengooborg queengooborg commented Aug 16, 2024

This PR fixes the mirroring script when ranges are involved. Apparently, if the upstream release was a range, the downstream browser would not be a range, which meant that various bits of data was not set to mirror when it should. This PR fixes this by introducing some logic that checks if the previous release (before the matching release) uses the same engine as the upstream browser, and if the upstream has a range, it adds the range delimiter.

The engine check for the last release is to help remove range delimiters where there not needed. For example, if the resulting version is Edge 79, we know that it's pointless to make it a range ("≤79") since Edge 18 and earlier used a different browser engine. As such, this change now produces the following:

  • Chrome 80 -> Edge 80
  • Chrome ≤80 -> Edge ≤80
  • Chrome 79 -> Edge 79
  • Chrome ≤79 -> Edge 79
  • Chrome 24 -> Edge 79
  • Chrome ≤24 -> Edge 79

Caveat: due to this change, some data can no longer be mirrored for Firefox Android and Opera Android. This is a minor caveat, however, since they'll be automatically fixed once the ranges are replaced with proper version numbers.

Fixes #23855 (closes #23856).

@github-actions github-actions bot added infra Infrastructure issues (npm, GitHub Actions, releases) of this project data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS data:html Compat data for HTML elements. https://developer.mozilla.org/docs/Web/HTML data:svg Compat data for SVG features. https://developer.mozilla.org/docs/Web/SVG scripts Issues or pull requests regarding the scripts in scripts/. data:wasm Compat data for Web Assembly features. https://developer.mozilla.org/en-US/docs/WebAssembly labels Aug 16, 2024
@queengooborg queengooborg requested a review from Elchi3 August 17, 2024 08:22
@Elchi3
Copy link
Member

Elchi3 commented Aug 19, 2024

This PR needs to be reviewed with the approach proposed in #23856.

@queengooborg
Copy link
Contributor Author

Ah, I forgot there was an existing PR that also did this, or at least something similar.

Since Philip is now away and won't be back for a long while, I think that we should close that PR and combine its changes into this one. I say this because I'd request changes on some of the code for both bug fixes and improved logic.

One key difference between that PR and this one is that it bases whether to add the range delimiter if the downstream version is the first release, but this PR only adds the range delimiter if the downstream version is the first release of the matching engine version, which means that less ranges are introduced during mirroring. (In Philip's PR, Chrome 23 would still become Edge ≤79 during mirroring.)

@github-actions github-actions bot added the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Oct 14, 2024
Copy link

This pull request has merge conflicts that must be resolved before it can be merged.

@queengooborg queengooborg requested a review from caugner October 14, 2024 20:13
@github-actions github-actions bot removed the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Oct 14, 2024
@github-actions github-actions bot removed the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Nov 2, 2024
caugner
caugner previously requested changes Nov 5, 2024
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, just three comments to make the implementation a bit clearer. And could we add a unit test?

scripts/build/mirror.ts Outdated Show resolved Hide resolved
scripts/build/mirror.ts Outdated Show resolved Hide resolved
scripts/build/mirror.ts Outdated Show resolved Hide resolved
queengooborg and others added 3 commits November 17, 2024 14:54
Co-authored-by: Claas Augner <[email protected]>
Co-authored-by: Claas Augner <[email protected]>
Co-authored-by: Claas Augner <[email protected]>
Copy link

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Nov 18, 2024
@github-actions github-actions bot removed the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Nov 18, 2024
@github-actions github-actions bot added the data:webext Compat data for Browser Extensions. https://developer.mozilla.org/Add-ons/WebExtensions label Nov 19, 2024
@github-actions github-actions bot added the size:l [PR only] 101-1000 LoC changed label Nov 22, 2024
@queengooborg queengooborg requested a review from caugner December 1, 2024 09:41
@queengooborg
Copy link
Contributor Author

Since the changes requested have been applied, I'm going to go ahead and merge this since an LGTM was given!

@queengooborg queengooborg merged commit 59a9a6a into main Dec 3, 2024
9 checks passed
@queengooborg queengooborg deleted the mirroring-with-ranges branch December 3, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS data:html Compat data for HTML elements. https://developer.mozilla.org/docs/Web/HTML data:svg Compat data for SVG features. https://developer.mozilla.org/docs/Web/SVG data:wasm Compat data for Web Assembly features. https://developer.mozilla.org/en-US/docs/WebAssembly data:webext Compat data for Browser Extensions. https://developer.mozilla.org/Add-ons/WebExtensions infra Infrastructure issues (npm, GitHub Actions, releases) of this project scripts Issues or pull requests regarding the scripts in scripts/. size:l [PR only] 101-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mirroring ranged versions removes ranges for most browsers
3 participants