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

Configlet fmt config.json files #292

Merged
merged 2 commits into from
Oct 25, 2023
Merged

Conversation

BNAndras
Copy link
Member

The latest configlet beta extends fmt to also work on the track config.json. I also patched the relative path for example.scm so sync puts it in the expected location inside .meta and not the root of the exercise.

@guygastineau
Copy link
Contributor

Did you need to pass special arguments to configlet fmt to get it to handle the config.json? We generate the config from track.ss, although the existing one is loaded first from GitHub if it isn't already present in an untracked directory called closet. that means, it is okay to add things like you are adding to it here directly to the json file, but the track specific information is all added based on file data in sexp format. If we should have configlet fmt handle the formatting of the config.json file, then we should probably have make-config in code/track.ss call configlet fmt to ensure we don't accidentally make horrible noise in PRs. Let me know if my explanation has made any sense. I am just reviewing during short work breaks right now.

@BNAndras
Copy link
Member Author

./bin/configlet fmt warns if the config.json is broken. ./bin/configlet fmt -u or ./bin/configlet fmt -uy will format the track config, but that seems a bit dangerous since it'll also format other unformatted exercise configs. There doesn't seem to be a good way to format just the track config.

@guygastineau
Copy link
Contributor

That makes sense. We should just put that in some part of the Makefile, I think, but we don't need that for this PR to go through.

@guygastineau
Copy link
Contributor

Please make an issue about it if you wish, or I can make one later. Thanks for your work on this PR!

@guygastineau guygastineau merged commit 2440072 into exercism:main Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants