exercism / cli

A Go based command line tool for exercism.org.
https://exercism.org/docs/using/solving-exercises/working-locally
MIT License
1.33k stars 359 forks source link

Implement behavior to migrate legacy exercises on the filesystem #639

Open NobbZ opened 6 years ago

NobbZ commented 6 years ago

Is atool as described in the subject planned, available or otherwise easily possible or not wanted at all?

kytrinyx commented 6 years ago

I was originally going to write this, but I wasn't sure how many people have extensive local copies to migrate. It sounds like it would be useful!

This would probably be pretty straight-forward to do with a bash script / powershell script.

kytrinyx commented 6 years ago

Thinking about this a little bit more I think I'd like to have this as a command in the CLI, so that we can simply call the "download" endpoint, which will return all the necessary data for the metadata file, and then we can simply not download the other files. Then people wouldn't have to be careful about backing up local copies etc.

kytrinyx commented 6 years ago

I spent a bit of time sketching out a few different possibilities, and the approach I like the best is to write an exercism doctor command which will walk the workspace and migrate any unmigrated exercises.

kytrinyx commented 6 years ago

This can rely on the the workspace.PotentialExercises method. https://github.com/exercism/cli/blob/80f08fce28e1061d66f55e4e5c8d8e7a27df3420/workspace/workspace.go#L48

The basic behavior should be:

if directory has solution, do nothing Test setup: an exercise directory with a metadata file Verify: it doesn't get overwritten

if directory has no solution, write it It should not overwrite any existing exercise/solution files. Test setup: a text file with different text than test server Verify: text file doesn't get overwritten, metadata file gets written

migrates all available exercises Test setup: create two fake tracks with no metadata, one with one exercise, one with two exercises Verify: they all got migrated, and STDERR contains a report of what was done

provide dry-run capabalities Test setup: same as "all available", but passes --dry-run flag Verify: none got migrated, STDERR contains a report of what would have been done

jdsutherland commented 6 years ago

I plan to give this a go this weekend.

jdsutherland commented 6 years ago

An initial thought: the convention for doctor commands in other CLIs I'm aware of is that it's a query outputting a list of potential problems. The two pending features (here and #699) are side effects. I'm not sure how wide this convention is but it might be surprising to users expecting a query.

What do you think about considering another name? groom or spruce come to mind but I'm no CLI lexicographer.

kytrinyx commented 6 years ago

That's a good point.

How about doctor which would only report, and then --fixup which would have side effects?

jdsutherland commented 6 years ago

Sure, --fixup works.

if directory has no solution, write it It should not overwrite any existing exercise/solution files. Test setup: a text file with different text than test server Verify: text file doesn't get overwritten, metadata file gets written

Is it necessary to fake a test server here? Would writing a text file and comparing ModTime() to verify it doesn't get modified accomplish the same ends?

Edit: After diving further into this, it seems the purpose is to verify that non-metadata files aren't downloaded (and thus the local workspace files aren't modified), so it seems that something like fakeSubmitServer is needed (and the above use of ModTime isn't going to cut it).

jdsutherland commented 6 years ago

... we can simply call the "download" endpoint, which will return all the necessary data for the metadata file, and then we can simply not download the other files. Then people wouldn't have to be careful about backing up local copies etc.

It seems we need most of runDownload which suggests some refactoring. Since both cmd/download and cmd/doctor need to call the "download" endpoint, what do you think about extracting the logic for getting a downloadPayload outside of cmd/download?

jdsutherland commented 6 years ago

I made a WIP PR https://github.com/exercism/cli/pull/744. Since there are multiple issues related to doctor, it's probably best to centralize the discussion to the PR.