affine cipher: remove reference to an exit code #1614
Conversation
There was a problem hiding this comment.
There is no reason to bump the version when changing only the description.md; that will just cause unnecessary CI churn on tracks with test generators.
Also exceptions are not idiomatic in all languages any more than error codes. Saying that the code must error would be more general.
|
re: bumping the version: I quote from README.md:
While that policy remains in effect, I believe it requires this patch version bump. I agree that "must error" is more general, and will make that change. |
While it is idiomatic for shell scripts to indicate an error status via exit code, doing so is uncommon in other languages. For generality, the problem specifications should not instruct the test suites to return an exit code. This includes a bump to the patch version as the test suite has not changed at all.
coriolinus
force-pushed
the
affine-cipher-remove-exit-code
branch
from
6d7381c to
44475c5
Compare
Also quoting from the [README.md]:
It has been my understanding that “test data” is the canonical-data.json, not the description.md, which does not contribute to the tests. |
|
To be more precise I believe you’d have to update the description or comments field within the test data to make a version able change to the test suites. |
|
I disagree: it seems straightforward that changing We need other people to cast their votes. |
|
Fair enough, but it’s not been just my understanding. Lots of changes to the description.md and metadata.yml have occurred without corresponding version changes in the actual test data. |
|
It seems that historically we have not considered I wonder if there is a Remember that handy script https://github.com/exercism/problem-specifications/blob/master/bin/check_versions that reminds us to change the version when it is necessary to change it? Its only condition that will cause it to say a version change is required is if a Well! Those serve as hints of what the interpretation has historically been, but not what the interpretation should be in the future. For that, I am sure any well-reasoned decision will be acceptable, and will allow the interested parties to have a proper discussion about what that well-reasoned decision will be, without any interference from me. I will note that the benefit of a version number for the JSON file is it helps tracks determine whether their tests are up to date, as tests are not generated in the same way for every language. In contrast, READMEs are generated in the same way for every language, so a working procedure for determining whether READMEs are up to date is "try to generate them all, and see if any of them changed". So it is not necessary to use the JSON's version number as a signaling mechanism for a README change, but surely there could be any other reason for wanting to change it. |
|
My argument against versioning the description.md and the metadata.yml is that changes in neither materially effect the task the student is performing... the canonical-data.json IS that task, so if a change to the copy implied anything more than a clarification or rewording of the task THEN it would necessarily mean updating the canonical-data.json and therefore trigger a version ratchet.
When/if we get around to considering i18n tokens in the README.md (as opposed to say a README.md.[lang]) then I’d consider the virtue of versioning it, though likely separately from the canonical-data.json.
…On Oct 24, 2019, 11:05 +0100, Peter Tseng ***@***.***>, wrote:
It seems that historically we have not considered description.md changes to prompt a change to the tests' version.
I wonder if there is a git command that is of the form "show me commits that modified description.md that did not modify canonical-data.json" . The best idea I have right now is a manual inspection of git log --stat exercises/*/description.md exercises/*/canonical-data.json. Since we don't like having to force people to do manual work, let's try a different tack.
Remember that handy script https://github.com/exercism/problem-specifications/blob/master/bin/check_versions that reminds us to change the version when it is necessary to change it? Its only condition that will cause it to say a version change is required is if a json file changed; .md files are not inconsideration. If we wanted .md files to cause a version change, I would guess that the script would have written in such a way.
Well! Those serve as hints of what the interpretation has historically been, but not what the interpretation should be in the future. For that, I am sure any well-reasoned decision will be acceptable, and will allow the interested parties to have a proper discussion about what that well-reasoned decision will be, without any interference from me.
I will note that the benefit of a version number for the JSON file is it helps tracks determine whether their tests are up to date, as tests are not generated in the same way for every language. In contrast, READMEs are generated in the same way for every language, so a working procedure for determining whether READMEs are up to date is "try to generate them all, and see if any of them changed". So it is not necessary to use the JSON's version number as a signaling mechanism for a README change, but surely there could be any other reason for wanting to change it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
I'm going to put this on hold for now. We're planning on making changes to how things are structured that consider exactly this thing. I'm hoping to get an issue written up asap, but other things keep getting in the way. |
|
Looking at the current description, the proposed fix appears to have already been applied. |
5 participants
While it is idiomatic for shell scripts to indicate an error status
via exit code, doing so is uncommon in other languages. For generality,
the problem specifications should not instruct the test suites to
return an exit code.
This includes a bump to the patch version as the test suite has
not changed at all.