Before this release, configlet fmt only operated on exercise .meta/config.json files.
With this release, configlet fmt also operates on exercise .approaches/config.json and .articles/config.json files.
But I cannot find information of what is expected or what should be done to fix it. Is it a problem with configlet fmt that it requires files to be there which weren’t there before, or is the problem that the files are really missing?
Would it make sense to create a non-breaking change first, perhaps some warning messages to allow the track to get up to standard and then make the breaking changes?
Ah, you are right @MatthijsBlom it is “not formatted”. I still don’t know what it means. I checked the first one from the list 33 files which are “not formatted” and it looks OK, it passes an online validation against RFC 8259 standard.
The files haven’t changed recently and are not part of the PRs which they break.
Previously the .approaches and .articles directories were not covered by the formatting check. Now they are, and it is discovered that the formatting is not in agreement with configlet’s sensibilities.
That’s why I pointed out above that it would be better to have some grace period, let it complain but not break builds to have a chance to improve things.
Also, it complaints that it is “not right” but offers absolutely no hints as to what can be done to make things better. I’m trying to learn Nim right to the point where I can read enough of the configlet code to figure out what’s going on.
Context: the csharp track had opted in to running configlet fmt in CI:
which means the track maintainers have explicitly said “we want CI to indicate failure if configlet fmt complains about a file’s formatting”.
With configlet 4.0.0-beta.11, configlet fmt learned to format more files. This breaks CI for any track that both:
opted in to formatting checks
and had at least one approach/article that was not formatted as configlet likes
However, only csharp and fsharp needed changes, and Erik is a maintainer of both. So CI wasn’t going to stay red for long.
I checked that configlet fmt was happy with the formatting on the other potentially affected tracks. Tracks that opted in to configlet fmt in CI:
$ cd /path/to/dir-containing-every-exercism-track/
$ rg -uu --sort path -g '*/.github/workflows/configlet.yml' --files-with-matches 'fmt: true' \
| cut -d'/' -f1
csharp
elixir
elm
fsharp
javascript
nim
tcl
unison
zig
Would it make sense to create a non-breaking change first, perhaps some warning messages to allow the track to get up to standard and then make the breaking changes?
Configlet has used deprecation periods and warn-before-error in the past. But I don’t think it’s useful here: at most we should’ve just PRed to every affected track at the time of configlet release. We often do that. But in this case:
The number of affected tracks was very small.
We want configlet fmt -u to operate on the newly supported files.
It’s simplest if the set of files that configlet fmt -u operates on is the same as the set of files that configlet fmt may produce an error for.
Also, it complaints that it is “not right” but offers absolutely no hints as to what can be done to make things better.
Yes, I can see that the CI output is non-obvious - especially if you’ve never used configlet. Thanks for the feedback. We can think about adding some more explanation/recommendations to the output. But in general, configlet is an Exercism-specific tool, written first for maintainers, and adding functionality is currently a higher priority than general polish.
Thanks, ee7, for the detailed explanation and the context.
I agree with most of what you said, except for the last bit. Developer experience, especially in open source projects is not ‘general polish’. It is important functionality.
@ErikSchierboom is the maintainer on the two affected tracks. I am trying to contribute a little to C# track without his knowledge, in my spare time. The change breaks make builds, my PRs. One extra line in the output, something like consider running configlet fmt -u would allow me to spend those two extra hours on improving the track, we wouldn’t have this discussion here saving more time to others. It’s not a very big deal, but it is not just “polish”.
Again, thanks @ee7-1282 for taking the time to provide the explanations, and thanks to @MatthijsBlom and @IsaacG for helping me get unbloacked earlier.
