Switch config to camelCase and support automatic migrations #1202
Conversation
This file contains 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
This was referenced Mar 15, 2024
Comment on lines
+103
to
+123
| export function migrateV1ConfigImpl(doc) { | ||
| // see here for more info about the visitor: https://eemeli.org/yaml/#finding-and-modifying-nodes | ||
| let hasChanged = false; | ||
| Yaml.visit(doc, { | ||
| Pair(_idx, pair, _path) { | ||
| // A previous version of the config used underscores in the config keys, | ||
| // but we standardised on camelCase instead. | ||
| // So here we crawl through the keys of each YamlMap and convert all the keys | ||
| // to camelCase. | ||
| if (pair.key && Yaml.isScalar(pair.key)) { | ||
| pair.key.value = pair.key.value.replaceAll(/_./g, function (match) { | ||
| hasChanged = true; | ||
| return match.charAt(1).toUpperCase(); | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
| if (hasChanged) { | ||
| return doc; | ||
| } | ||
| } |
There was a problem hiding this comment.
So, this effectively disables snake_case entirely from the config forever. Moreover, even if someone decided to add a superfluous key to the YAML file in that particular case convention, this script will force it to be camel-case, even if that's not what the user wants. I think that's fine.
There was a problem hiding this comment.
if someone decided to add a superfluous key to the YAML file in that particular case convention, this script will force it to be camel-case, even if that's not what the user wants
Once we find a way to go at #1165 then I think we could effectively disallow superfluous fields, they only seem to cause confusion (see the comment I left in the Config module)
JordanMartinez
approved these changes
Mar 15, 2024
m15a
added a commit
to m15a/purescript-choku
that referenced
this pull request
May 10, 2024
Now config uses camel case: purescript/spago#1202
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 PR moves the config format to be consistently camelCase, to fix #1139
As we figured in #1154, this is a big change for the few projects out there that already use the new spago, so we needed to (a) be backwards compatible, and (b) support automatic migration of the config.
The previous attempt to support this was #1168, but that got convoluted enough that we agreed on a simpler approach - here we manipulate the YAML doc entirely on the JS side, and write it to disk if anything was changed, and the user requested it (otherwise we just warn about the config format being out of date)