-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add mirroring script #4729
Add mirroring script #4729
Conversation
Hey @AdaRoseCannon, you might be interested in this PR. |
Co-Authored-By: Florian Scholz <[email protected]>
I think it would be a great improvement if the mirroring script would also print out a report of notes (ideally with links to the files and lines of the notes), so that the PR author can paste that report into the PR description and manually double check all notes reported before submitting the PR. This can be a follow-up after the initial landing of this project, though. |
Review comments have been addressed, thank you! |
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.
Some more suggestions to the markdown. Sorry, tech writer reviewing here :)
(I didn't run prettier, but I hope the suggestions pass, might need to correct if not)
Thank you so much for moving things around! This is so much better! I'm quite happy with how it is now.
This is very true. I have now reviewed each bump function and I think they all look good so far. The one for Edge is a bit more complex, but this is also the one we've paid a lot of attention to when we recently updated all the Edge values and it seemed to do the right thing. So, I think we can merge this, but should still continue to pay attention to the PRs generated by this script. Having this merged and "officially" supported is not a blind guarantee that generated data is always correct. To any reviewer: Please especially review notes and corner cases (version_removed, prefixes, flags, array statements and such things come to mind). The mirroring script does seem to do a good job for all of these things, but you never know basically. Of course we will fix bugs as we see them, and I think this script is now tidied up to allow extending it further as needed. |
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 think I found a bug with the refactoring.
This works! If we can get the suggestions to the markdown file in, then I will merge this \o/ |
Co-Authored-By: Florian Scholz <[email protected]>
Co-Authored-By: Florian Scholz <[email protected]>
Oops, didn't see the documentation suggestions -- applied! |
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.
Thank you for 97 commits of patience, many months of field testing and getting this over the finish line now! This will be sooo enormously helpful! Cheers! 🎉 🥇 💯
This PR is to aid in the efforts of #4719. It adds a new script designed to help eliminate inconsistencies and mirror data between browsers. The script started off as a carbon copy of the Python script I had written, but since has been enhanced with numerous new features. For example, command line arguments that allow the choice of a specific source browser, and to allow for forcing a mirroring on compatibility statements that aren't
true
ornull
. Furthermore, rather than using a static mapping of browser, a function has been written to dynamically retrieve said mappings, by finding the matching engine and the first release with an equal or greater version number, orfalse
if one cannot be found.This script also contains a more complex mirroring for Chromium-based Edge, to append the existing data rather than overwrite everything. (See #5214 (comment) for a description of the logic.) To use this Chromium-based mirroring, run
npm run mirror edge [feature_or_file] -- --source=chrome --modify=always
.This script has undergone lots of field testing and has already created various merged PRs. While most of the bugs have been eliminated, there may still be a few lurking around in the corners. Let me know if you find a bug and I'll fix it right away!
Planned features after initial merge: