feat(core/config): allow close to ignore for migration PRs

[Gabriel-Ladzaretti]

Changes

Currently a new migration PR would be created regardless of a closed one.

Now if we detect a closed migration pr, an Ignore Comment is added to the PR and we won't creating a new one.

Context

closes #16774

Documentation (please check one with an [x])

• I have updated the documentation, or
• No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

• Code inspection only, or
• Newly added/modified unit tests, or
• No unit tests but ran on a real repository, or
• Both unit tests + ran on a real repository

[Gabriel-Ladzaretti]

ladzaretti/feat-config-migration#2

[viceice]

i think a config migration PR should be immortal.

@rarkins WDYT?

[rarkins]

i think a config migration PR should be immortal.

@rarkins WDYT?

I think it's ok for us to have "close to ignore". Presumably that's what any user would intend if they close it.

Important though that a merged migration PR does not block a new one

[viceice]

but wasn't the plan to remove old migration code from renovate when all repos are migrated some time in future?

if we allow to ignore those migrations, we will break them when removing the migration code.

that's why I think the migration PR should be immortal.

[rarkins]

but wasn't the plan to remove old migration code from renovate when all repos are migrated some time in future?

Yes, although who knows how achievable that will be. We might just end up removing selective migrations if for example we want to reuse an old config option name, while leaving the rest.

if we allow to ignore those migrations, we will break them when removing the migration code.

that's why I think the migration PR should be immortal.

Yes, but that's their choice. It reminds me a little about people who insist on ignoring Windows updates requests forever. Windows keeps popping up requests and those people get angrier.

Maybe we just let them close it but add a comment which makes it clear that their config may break in future, in which case they'll get a config validation issue instead.

[viceice]

They can disable config migrations already (when it's enabled by default some time in future), that would block new PR too 🤷‍♂️

[rarkins]

They can disable config migrations already (when it's enabled by default some time in future), that would block new PR too 🤷‍♂️

I had thought the same, and for example same about dependency dashboard issues. But then the other day one user was practically outraged that when they closed the dashboard issue it came back again. So now I lean towards letting the user to be the boss, even if we think it's unwise or unreasonable.

One day we'll turn on migrations by default. There's definitely going to be some people who think "no thanks" and close the migration PR. The question is, do we or they gain anything by deciding to ignore that signal and reopening the PR until they either get so annoyed they uninstall, or figure out how to disable using config?

[viceice]

Add a description and a link to the PR body how to disable?

[Gabriel-Ladzaretti]

Add a description and a link to the PR body how to disable?

🔕 Ignore: Close this PR and you won't be reminded about this update again.

like we do for other PRs ?

[rarkins]

Needs to say something like "but one day your current config may no longer work"

[Gabriel-Ladzaretti]

ignore message added -

ladzaretti/feat-config-migration#5

[rarkins]

We have concluded that we will allow users to close to ignore

[viceice]

See comment

[Gabriel-Ladzaretti]

fixed error due to merging main.
can we try merging this again 😄?

[Gabriel-Ladzaretti]

anything else to do here 😀?

[renovate-release]

🎉 This PR is included in version 32.169.0 🎉

The release is available on:

• GitHub release
• 32.169.0

Your semantic-release bot 📦🚀