Update README

[rainij]

The README.md is heavily outdated, I essentially rewrote it.

[ErikSchierboom]

Let me know when you want me to review it.

[rainij]

Let me know when you want me to review it.

Yes of course, I just want to have a look into some of the more popular tracks to look what they write in their README and similar files. I will probably have time for that on the weekend.

[rainij]

I recently added some exercises and it took me some time to find out what to do. The main reasons are that the current sml-README is outdated and that the exercism wide documentation is rather verbose. Therefore I added a comprehensive section on this topic with concrete steps to follow.

To generate the "boilerplate" when adding an exercise I used the following three commands (after modifying the repo-level config.json), which I added to the README:

$ bin/configlet sync --update --tests include -e {{ slug }} # 1. Add tests.toml including all available tests.
$ bin/configlet sync --update -e {{ slug }}  # 2. Interactively add required files with "generic" content.
$ bin/generate {{ slug }}                    # 3. Autogenerate required files with sml-specific content.

I would have liked it to execute configlet only once and without any interactive prompts since I suppose yes is right most of the time. But for me there were two inconveniences (just my personal opinion):

1. --yes never works if --tests is explicitly specified, not even with --tests include where configlet still complains about the mode choose. Example:

$ bin/configlet sync --update --docs --filepaths --metadata --tests include --yes -e beer-song
Error: '-y, --yes' was provided to non-interactively update, but the tests updating mode is still 'choose'.
...

Don't know if this is a bug?

2. To avoid interactivity in step 2 one can use --yes but only if one specifies --docs --filepaths --metadata too. Note that I actually only want to exclude --tests because of incompatibility with --yes. Hence I do not like to give all these options explicitly because I feel this to be a little more brittle against future changes on the tool.

@ErikSchierboom So in case I do something wrong just correct me and I will change the part in the README accordingly.

[ErikSchierboom]

Don't know if this is a bug?

I'll look into it, but it is definitely not ideal (CC @ee7) I've ran into this myself too. I've opened an issue: https://github.com/exercism/configlet/issues/673

[ee7]

this is a bug?

Yes. Thanks for the write up. It'll be fixed soon.

2. To avoid interactivity in step 2 one can use --yes but only if one specifies --docs --filepaths --metadata too. Note that I actually only want to exclude --tests because of incompatibility with --yes.

Indeed, there was a tradeoff here. Essentially, the configlet sync design came about because:

• We wanted one command, sync, to handle syncing of every kind of data
• We wanted syncing every kind of data to be the default
• We wanted to support interactive and non-interactive use
• We wanted making destructive changes to require confirmation or extra flags

But one of the syncing kinds, tests, is different from the others: the decision is between three options "make changes in one direction, make changes in the opposite direction, or do nothing" rather than "make changes or do nothing".

So the choice seemed to be between:

1. The current situation, with --tests choose|include|exclude

Update only tests, including all new test cases:

configlet sync -u --tests include

Update docs and tests, including all new test cases:

configlet sync -u --tests include -y --docs

Update everything, including all new test cases:

configlet sync -u --tests include -y --docs --filepaths --metadata

2. Make --tests not take an argument, and add a -tests-mode choose|include|exclude option

Update only tests, including all new test cases:

configlet sync -u --tests-mode include --tests

Update docs and tests, including all new test cases:

configlet sync -u --tests-mode include --tests -y --docs

Update everything, including all new test cases:

configlet sync -u --tests-mode include -y

---

The current design makes it shorter to update some subset (the first two tasks), at the cost of requiring explicitly specifying each data kind when updating everything in one command (the third task).

Hence I do not like to give all these options explicitly because I feel this to be a little more brittle against future changes on the tool.

If configlet sync starts to sync a new kind of data in the future, it's not guaranteed that answering "yes" is a good default decision, or a valid one (if it is not a binary yes/no decision). So the current design, requiring explicit selection of the syncing scope when updating more than one kind of data, may be less brittle overall: adding an extra data kind is a non-breaking change that does not change the behavior of existing scripts.

That is, if we write a script now that tries to "update everything", maybe it's better that it doesn't automatically start doing something extra in the future. And for each hypothetical new data kind that does produce a binary choice, maybe it's better that we add --foo a|b|c rather than --foo --foo-mode a|b|c.

I too feel that it's annoying to type more when updating everything. But configlet has completion scripts now, distributed in the next release, which lessens the pain somewhat.

Any thoughts? I appreciate the feedback - there hasn't been that much.

[rainij]

Thanks @ee7 for the explanations and rationale behind confglet. Might be true that it is less brittle then I think.

I suppose that if a new kind of files will be added then probably configlet lint would check if they are present (in case they are required).

So it is just a little bit inconvenient to specify all these options. But this is probably acceptable and one might even argue that it is good to think about what one really wants :smile: .

I would also be against a separate --test-mode option, even if it solves my second issue somehow I would not like it since it looks more complicated in some sense.

Just in case somebody still considers to reduce the verbosity. My first impulse was that something like this would have been nice:

• explicitly stating that one wants everything (--all) by configlet sync -u --all and not implicitly by configlet sync -u,
• overwrite the defaults like so configlet sync -u --all --tests include -y,
• dealing with the boolean options by allowing an optional arg like so configlet sync -u --all --tests include --docs no -y (or alternatively --no-docs).

But as said, this is just a "first impulse". I did not carefully think about it and I am hence I don't know if it was a good idea. And: I definitely don't want to produce needless work for you - just a random thought for you information :smile: .

I think fixing https://github.com/exercism/configlet/issues/673 is really sufficient for now. If it is quickly merged I might even change the README accordingly within this PR.

[ee7]

Thanks for your thoughts.

• explicitly stating that one wants everything (--all) by configlet sync -u --all and not implicitly by configlet sync -u,

This one won't happen, because:

• We want a plain configlet sync to exit with a non-zero status if any data kind is out of date.
• We want consistency between configlet sync and configlet sync -u

• dealing with the boolean options by allowing an optional arg like so configlet sync -u --all --tests include --docs no -y (or alternatively --no-docs).

I think a few power users might find configlet sync -uy --no-tests a handy shortcut (to sync docs, filepaths and metadata), but the options would become too convoluted. Especially since we also have --yes, and the no part of --no-tests means something different.

So I'm afraid the best I can suggest is some shell alias like this:

alias configlet_sync_all='configlet sync -uy --docs --filepaths --metadata --tests include'

and/or the shell completion scripts in the new configlet release.

I think fixing exercism/configlet#673 is really sufficient for now. If it is quickly merged I might even change the README accordingly within this PR.

The bug should be fixed in the new release, configlet 4.0.0-beta.6. Please let me know if there's still a problem after re-running fetch-configlet.

[rainij]

@ee7 the new release of configlet works fine, I am able to run the formerly forbidden combination of options. I updated the README of this repo accordingly.

[rainij]

@ErikSchierboom updated the README in response to the already mentioned bugfix in configlet. This MR can now be reviewed (but it is not urgent of course).