-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(config): migrate inline with the same sort #12091
feat(config): migrate inline with the same sort #12091
Conversation
Are you planning to rewrite every single migration rule in one go? I would prefer to avoid that if so - too much risk of missing something |
8d9a6b9
to
aea61bd
Compare
Nope, I guess in one go it will be too huge PR. If you agree with idea, I will move step by step. |
OK please get one step working first to demonstrate. I was hoping you would use the code I posted earlier which solves the majority of the problem in one go |
aea61bd
to
2db3eb9
Compare
2db3eb9
to
c719277
Compare
From our Pull Request template:
|
https://github.com/renovatebot/renovate/pull/12091/files#diff-6c6bef2e9a0f4b1863006b6c796698bedb832302c1c56b147e6471ae043684eaR31 this test checks, that MigrationService saves original order of configuration properties Also I have added examples for different migrations |
|
When you test, it's best if you can include some non migrated config too, so that we can observe they are ordered correctly even if not migrated. |
@rarkins sorry, I did not get it. I have created repository with non migrated config https://github.com/pret-a-porter-testing/renovate-reproduction-11459. On screenshot you can see lines with |
Yes, you need to do something else. Every config option in newConfig has been migrated. You should test with a mix of migrated and non-migrated (does not need migration) config. |
@rarkins done
Also you can provide some more specific config for testing |
Can you put ignoreTests last and verify that it remains last? |
Actually, have a mix of non-migrated fields first, last and in the middle of migrated and verify ordering remains as expected |
|
@rarkins shall I do any other validations regarding this PR? |
This test doesn't interleave migrated and non-migrated. It has all migrated first, then non-migrated. Can you e.g. move |
@rarkins it works fine |
🎉 This PR is included in version 29.12.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Changes:
Refactor migrations to provide ability to migrate and save original order of properties in configuration
Context:
#11459
Documentation
How I've tested my work
I have verified these changes via: