Check if configuration is valid prior to attempting migration #4334
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChrisMcD1
commented
Feb 27, 2025
| } | ||
|
|
||
| content, err = migrateUserConfig(path, content) | ||
| existingCustomCommands := base.CustomCommands |
Contributor
Author
There was a problem hiding this comment.
This line needs to be before we make any potential modifications to base , so I have just moved it fully before the parsing.
This means that we can make the migration step as expensive as necessary, since we will no longer run it on every app startup.
ChrisMcD1
force-pushed
the
check-configuration-prior-to-migration
branch
from
508d2fd to
7154cda
Compare
7 tasks
Contributor
Author
|
Closing for reasons stated in #4318 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a spinoff from discussion in #4318, and the solution works quite well.
The decoding now happens in 2 steps so that in theory if there is a fully invalid YAML document in some way, we don't even attempt the migration.
This PR ended up being pretty simple, and worked smoothly!
go generate ./...)