If you go to the link for the Update config.json entry, you are redirected to this page. That page has a set of things to do, and if those things are done, the item can be checked.
If you go to the link for the Update config.json entry, you are redirected to this page. That page has a set of things to do, and if those things are done, the item can be checked.
That means the "Updated config.json" entry is just for the initial migration of config.json, and so the intended README.md state after that migration is:
- [ ] Implemented 20+ Concept Exercises
- [x] [Updated `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [x] Added `version` key
- [x] Added online editor settings
- [x] Added `indent_style`
- [x] Added `indent_size`
- [ ] Added Concept Exercises
- [ ] Added Concepts for all Practice Exercises
However, this looks strange because the final two checkboxes are children of "Updated config.json", and usually, I would not tick a top-level checkbox until all its child checkboxes are completed. And indeed, most tracks don't check it.
Would it be clearer if we made a change to the README.md template? The expected README.md state after the initial migration could be something more like:
- [ ] Implemented 20+ Concept Exercises
- [x] [Migrated `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [x] Added `version` key
- [x] Added online editor settings
- [x] Added `indent_style`
- [x] Added `indent_size`
+ - [x] Converted the `exercises` array to an object with a `concept` key and a `practice` key
- - [ ] Added Concept Exercises
- - [ ] Added Concepts for all Practice Exercises
+- [ ] `config.json`: Added Concept Exercises
+- [ ] `config.json`: Added Concepts for all Practice Exercises
This better separates the initial migration from the later adding of exercises.
Edit: We could also move the
- [ ] Implemented 20+ Concept Exercises
down so that it comes after the checkboxes for migrating config.json. That better reflects the expected development order (unless a track implements 20 concept exercises before adding a config.json!).
Thanks for the feedback! @iHiD what do you think?
I've remove the config.json checkbox all together, and just have it as a bullet point with the sub-checkboxes nested below it.
Then remove - [ ]config.json: Added Concepts for all Practice Exercises as we haven't given people a spec for that yet.
And probably remove - [ ]config.json: Added Concept Exercises too as that's a per-exercise thing.
@iHiD Let me batch-script that as an update
@iHiD could you write the end result?
Is it also a bit overkill to have an individual checkboxes for each of the "migration steps"? Before I read your reply, I was going to suggest something like:
- [x] [Add initial `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [ ] Implement 20+ Concept Exercises (and add them to `config.json`)
- [ ] Add Practice Exercises to `config.json (and add concepts for them)
which agrees with your
And probably remove - [ ] config.json: Added Concept Exercises too as that's a per-exercise thing.
But then the todo list seems a bit short. Perhaps it could then be made more granular, adding sub-items to e.g. "implement concept exercise" instead.
This also changes it to use the imperative, matching what is generally considered good style for e.g. git commit messages.
From https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project:
Write your commit message in the imperative: "Fix bug" and not "Fixed bug" or "Fixes bug."
And just generally how people write todo lists:
https://orgmode.org/guide/Checkboxes.html
But then the todo list seems a bit short.
It will get long over time. This is the key thing about this list - that we want to be able to add things to it over time and that be obvious to maintainers.
could you write the end result?
- [Updated `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [x] Added `version` key
- [x] Added online editor settings
- [x] Added `indent_style`
- [x] Added `indent_size`
This also changes it to use the imperative, matching what is generally considered good style for e.g. git commit messages.
I'm 50/50 on this. Agree with imperative being good for commits, but that's for the purpose of replaying, which doesn't fit here. But I can see the argument for why it's still good. I think it's probably not worth the hassle of changing that 6 months in, as this won't live beyond the next 6 months.
@iHiD Did you mean this?
- [ ] Implemented 20+ Concept Exercises
- [Updated `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [x] Added `version` key
- [x] Added online editor settings
- [x] Added `indent_style`
- [x] Added `indent_size`
Otherwise we're left with the entire to-do list as just the steps for migrating config.json.
What did you think about:
Note that both your version and the current version is missing steps 3 and 4 from the migration document:
- Convert the "exercises" array to an object with two properties
and
- Remove the "foregone" property. The property will return once the Practice Exercises have been added.
So I think we should either:
Arguments for the latter:
version key, and then later a PR to add the online_editor settings, etc.But yes, we have more important things to think about. And I don't mind keeping the items in the past tense.
To summarise, what do you think about this?
- [x] [Added initial `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [ ] Implemented 20+ Concept Exercises (and added them to `config.json`)
- [ ] Added Practice Exercises to `config.json`
Or maybe without the last item, given that you wrote
Then remove
- [ ] config.json: Added Concepts for all Practice Exercisesas we haven't given people a spec for that yet.
What if we did:
- [Migrate `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [x] Added `version` key
- [x] Added online editor settings
- [x] Added `indent_style`
- [x] Added `indent_size`
- Concept Exercises
- [ ] Implemented 20+ Concept Exercises
- [ ] Added Concept Exercises to `config.json`
- Practice Exercises
- [ ] Added Concepts for all Practice Exercises
This has the top-level elements as headers instead of being checkable. I do like the individual migration steps being listed as individual bullet points, as it a) helps people get a sense of progress, and b) allows for us to add to the list later on more easily.
That's what I mean, yeah. I don't have the brainspace to reason about this massively right now, so sorry for not replying @ee7. Erik's suggestion is inline with what I'm thinking if that works for you?
Should we leave the "added concepts for all practice exercises" out until we've actually got to the point where we've got a formal spec for them. I think that addresses some of the discrepancies that @ee7 raises.
Should we leave the "added concepts for all practice exercises" out until we've actually got to the point where we've got a formal spec for them
Yeah that would work!
Also, it may be redundant to have 2 checks under concept exercises; adding them to the config.json should be part of implementing the concepts exercises. Maybe we could add listing/ making issues for the concept exercises.
Something like this then?
- [Migrate `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [x] Added `version` key
- [x] Added online editor settings
- [x] Added `indent_style`
- [x] Added `indent_size`
- [ ] Implemented 20+ Concept Exercises
- [ ] Added Concepts for all Practice Exercises
I don't have the brainspace to reason about this massively right now, so sorry for not replying @ee7
No problem.
Replying to @ErikSchierboom's latest comment:
Something like this then?
- [Migrate `config.json`](../../docs/maintainers/migrating-your-config-json-files.md) - [x] Added `version` key - [x] Added online editor settings - [x] Added `indent_style` - [x] Added `indent_size` - [ ] Implemented 20+ Concept Exercises - [ ] Added Concepts for all Practice Exercises
I'm fine with this, or your earlier version with headings instead (with or without the - [ ] Added Concepts for all Practice Exercises checkbox, which @iHiD questioned).
But I still have a question from earlier: is there a reason why we don't add child checkboxes for steps 3 and 4 from the migration documentation?
- Convert the "exercises" array to an object with two properties
- Remove the "foregone" property. The property will return once the Practice Exercises have been added.
Otherwise it seems to me like the list is out-of-sync with the documentation.
I hadn't considered
b) allows for us to add to the list later on more easily.
which I agree could be a reason to keep the child checkboxes. But it seems like any new config.json items would be logically distinct from these initial "migration" steps, and so they shouldn't be under a "Migrate config.json" header anyway?
Below are some non-blocking thoughts. I'd be fine with merging a PR that applies either of Erik's most recent suggestions to all the README.md files; we've already addressed the problem raised in the title of this issue, as well as some others.
@ErikSchierboom's earlier version:
- [Migrate `config.json`](../../docs/maintainers/migrating-your-config-json-files.md) - [x] Added `version` key - [x] Added online editor settings - [x] Added `indent_style` - [x] Added `indent_size` - Concept Exercises - [ ] Implemented 20+ Concept Exercises - [ ] Added Concept Exercises to `config.json` - Practice Exercises - [ ] Added Concepts for all Practice Exercises
makes me wonder if it's more natural to move the items under ### markdown headings, rather than these "list item headings". At the moment, they're grouped under a ### Track Structure header.
In context, it could be something like:
## Readiness for Launch
Before launch, we need all of the following parts to be completed:
### config.json
- [x] [Migrate `config.json`](../../docs/maintainers/migrating-your-config-json-files.md)
- [x] Added `version` key
- [x] Added online editor settings
- [x] Added `indent_style`
- [x] Added `indent_size`
- [x] Added an `exercises` object with `concept` and `practice` keys <--- was missing?
- [x] Removed the `foregone` property (if it existed) <--- was missing?
- ^ or above, just the top-level checkbox?
- [ ] there's now space here to add stuff without it being under the "migration" heading
- [ ] stuff
- [ ] stuff
### Concept Exercises
- [ ] e.g. creating issues
- [ ] e.g. extracting concepts from v2
- [ ] e.g. list more concepts
- [ ] etc
- [ ] Implemented 20+ Concept Exercises (and added them to `config.json`)
### Practice Exercises
- [ ] Added Concepts for all Practice Exercises <--- @iHiD questions this, until we have a spec
- [ ] there's space here to add more Practice Exercise stuff
### Representer
- [ ] Build Representer
- [ ] Deploy Representer
### Test Runner
- [ ] Build Test Runner
- [ ] Deploy Test Runner
This might make sense if we expect that we'll add more list items. But splitting off config.json isn't fully convincing since the Concept Exercise and Practice Exercises sections also involve config.json. It partly depends what the aforementioned future extra checkboxes for config.json are.
Just an idea. I don't have the best sense of the expected future changes to README.md. And to reiterate: I'm sure that any PR made based on the previous discussion is good enough.
@ynfle PR #1731 aims to fix most of the issues.
Thanks everyone here - especially @ee7 for all your thoughts!
Most helpful comment
@ynfle PR #1731 aims to fix most of the issues.