Add possibility to retrieve canonical-data from local checkout of problem-specifications

[rainij]

I updated ./bin/generate so that on ./bin/generate [options] <some-exercise> it tries to get the canonical data from a local clone of problem-specifications first. Only if this fails (e.g. because it is not there) it tries to retrieve it from github. This resolves issue https://github.com/exercism/sml/issues/195.

EDIT: I added the option --problem-specs-source {remote,local} to the binary and changed the behaviour as discussed below.

[rainij]

@ErikSchierboom thanks for the feedback. I was also wondering if there might arise problems. Probably the root of the problem is that all this happens implicitly.

It may be an option to spend a flag like --problem-specs-source [local | online] or something like that. Making online the default to avoid the problem that was mentioned. This probably isn't "local-first" but maybe a raesonable compromise?

[ErikSchierboom]

@rainij It would be a good compromise I feel.

[rainij]

@ErikSchierboom thanks again for the immediate reply. I set the PR to draft again. I will have time for this soon (today).

[ErikSchierboom]

LGTM. @kotp can do the actual review

[rainij]

@kotp I tried to update the PR according to your suggestions. I also did another thing: I use enum.Enum instead of the strings local and remote now. I feel this is much safer. It took me some time to make enum and argparse work nicely together.

[rainij]

@kotp I observed too, that after applying generate to some exercise the exercise may be broken (i.e. make test-<exercise> fails). But this is already an older issue (unrelated to this PR).

Mostly just the name of the function to be tested is wrong/different (e.g. two-fer vs name, but this is not the only example). If I recall correctly the generate-script assumes that the property in canonical-data is the name of the function. It seems that this changed some time ago for some exercises. However, the file .meta/example.sml still uses the old name. For some reason in two-fer the function was called name previously and this is also what is used at the web-site (when someone tries the exercise). This seems to cause the error. And as I said, other exercises are affected too.

[rainij]

@kotp is there a plan when to merge this PR? Or is still some action from my side required? Or am I supposed to merge it by myself (think I am not allowed to at the moment)?

[kotp]

I was not aware that the author was not allowed to do so, once someone on the list "authorized" it through an approval.

I can do so now.

[kotp]

I can do so now... but really I prefer if you clean up the commit history, if you can, so that it is as you would like it to appear in the history.

[ErikSchierboom]

I can do so now... but really I prefer if you clean up the commit history, if you can, so that it is as you would like it to appear in the history.

Would a squash merge do?

[rainij]

@kotp would a single squashed commit with some meaningful commit-message suffice (as suggested above by your colleague)?

At my work we usually throw away the commits of a branch. We usually squash everything into one commit (if necessary for a rebase on master) or keep it as is if no (difficult) rebase is needed. But we try to write a good description of the PR/MR. Therefore I have no experience how a nice history on a working-branch should look like.

If you agree with a squash I can do it (probably tomorrow) and additionally a rebase on main. What do you think?

[rainij]

@ErikSchierboom @kotp I squashed everything into two commits and I did a rebase on main. I hope it is ready for merge now.

[kotp]

As long as the history is as you want it to be, I am good with it. I just knew that the WIP comments are not wanted, and that it was not done "If it means Work In Progress". With them gone, it is apparent that it is no longer in progress. You are the author of the history at this point, and doing a squash and not giving you an opportunity to present it as you want it would have been a little presumptuous.

[rainij]

@kotp thanks, I agree it is better now with a more or less clean commit-history. I keep this in mind for future PRs :) .