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

Add migration script for mirroring #16484

Merged
merged 1 commit into from
Jun 12, 2022
Merged

Add migration script for mirroring #16484

merged 1 commit into from
Jun 12, 2022

Conversation

foolip
Copy link
Contributor

@foolip foolip commented May 31, 2022

No description provided.

@github-actions github-actions bot added bulk_update An update to a mass amount of data, or scripts/linters related to such changes scripts Issues or pull requests regarding the scripts in scripts/. labels May 31, 2022
@foolip
Copy link
Contributor Author

foolip commented May 31, 2022

I haven't written any tests for this, because no matter the amount of tests I think they have no value. The proof that this is good is by comparing build/data.json before and after using it.

@foolip foolip requested a review from queengooborg May 31, 2022 09:44
@foolip
Copy link
Contributor Author

foolip commented May 31, 2022

This is able to replace ~75k support statements across 2000+ files, but I think the way to use this should be:

  • First node scripts/migrations/009-mirror.js --browsers chrome_android edge firefox_android opera opera_android safari_ios samsunginternet_android webview_android for the cases that can be mirrored in all browsers, no complications to look into
  • Then run things like node scripts/migrations/009-mirror.js --browsers chrome_android webview_android and node scripts/migrations/009-mirror.js opera opera_android and for smaller groups of things that will often be mirror-able
  • Finally go through mirroring the individual browsers. This is the step where a lot of odd discrepancies might be discovered.

@queengooborg
Copy link
Contributor

Could we set this to migration 009? I have a pull request for a migration 008 that's waiting on your review!

@foolip
Copy link
Contributor Author

foolip commented Jun 1, 2022

I've renamed to 009. But also, do we need the numbers? We're missing some in the series, and will also delete scripts that have served their purpose.

@queengooborg
Copy link
Contributor

Some migrations' scripts were deleted because they were just runs of npm run fix. But you're right; the numbers aren't too useful when compared to the review and merge timing. Let's opt to remove them in a future PR.

@foolip
Copy link
Contributor Author

foolip commented Jun 1, 2022

@queengooborg this is ready for review now.

Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

I haven't tried running the script, but this looks great to me -- let's start migrating browsers to mirror!

@queengooborg queengooborg merged commit e1dd58f into main Jun 12, 2022
@queengooborg queengooborg deleted the mirror-migrate branch June 12, 2022 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bulk_update An update to a mass amount of data, or scripts/linters related to such changes scripts Issues or pull requests regarding the scripts in scripts/.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants