ETL descriptions too long #1526
Conversation
exercises/etl/canonical-data.json
Outdated
| @@ -10,7 +10,10 @@ | |||
| "such that keys are integers. e.g. in JavaScript, it might look ", | |||
| "like `transform( { 1: ['A'] } );`" | |||
| ], | |||
| "description": "transforms the a set of scrabble data previously indexed by the tile score to a set of data indexed by the tile letter", | |||
| "description": "Transform", | |||
There was a problem hiding this comment.
Consider
| "description": "Transform", | |
| "description": "Transform score based index to letter based index", |
There was a problem hiding this comment.
Okay, a single word is a bit too minimal a description, but your suggestion for an identifier leaves my mind twisted and feeling its more a comment than a descriptor. I committed an alternative that hopefully meets you in the middle.
There was a problem hiding this comment.
In this case, why don't we just remove the nesting of the cases? I feel that it does serve any real purpose here, as grouping of test cases is usually only done when there are multiple groups (which there aren't here). So my proposal would be to just drop the nesting. What do you think?
exercises/etl/canonical-data.json
Outdated
| @@ -10,7 +10,10 @@ | |||
| "such that keys are integers. e.g. in JavaScript, it might look ", | |||
| "like `transform( { 1: ['A'] } );`" | |||
| ], | |||
| "description": "transforms the a set of scrabble data previously indexed by the tile score to a set of data indexed by the tile letter", | |||
| "description": "Transform legacy to new", | |||
| "comments": [ "Transforms a set of legacy scrabble data stored as letters per score", | |||
There was a problem hiding this comment.
There are now two comments fields on the same level.
There was a problem hiding this comment.
Oh woops. Missed that in the diff.
There was a problem hiding this comment.
@bencoman The new comment is great. Could you perhaps move it to the existing comment on the same level?
|
Lets hold further discussion here pending #1525 and #1473. Note, I've just discovered I accidentally committed direct to master and I'll need to clean that up once a path forward is clear. I was unfamiliar with using Github's web-editor and must have committed it directly before a second commit to a branch to issue this PR. |
That happens fairly easily with the web editor. Might be a good idea to setup branch protection for master on this repo to prevent accidental pushes without PRs. edit by @SleeplessByte: it's set up! Thanks @iHiD |
Such a long test is really a comment, not a description to be used as a test identifier. Make it so, per #1473
rpottsoh
force-pushed
the
bencoman-patch-1
branch
from
3dc5114 to
662b370
Compare
|
Errant commit has been reverted. |
exercises/etl/canonical-data.json
Outdated
| @@ -10,7 +10,10 @@ | |||
| "such that keys are integers. e.g. in JavaScript, it might look ", | |||
| "like `transform( { 1: ['A'] } );`" | |||
| ], | |||
| "description": "transforms the a set of scrabble data previously indexed by the tile score to a set of data indexed by the tile letter", | |||
| "description": "Transform legacy to new", | |||
| "comments": [ "Transforms a set of legacy scrabble data stored as letters per score", | |||
There was a problem hiding this comment.
@bencoman The new comment is great. Could you perhaps move it to the existing comment on the same level?
done |
Co-Authored-By: Erik Schierboom <[email protected]>
|
In future I'll use https://jsonlint.com/ to validate JSON before committing. |
|
https://jsonschemalint.com is also useful for checking your work against the schema. |
It seems that only a version bump is necessary. Latest changes to this problem: - 1.0.0 to 1.0.1, descriptions too long: exercism/problem-specifications#1526 - 1.0.1 to 2.0.0, change input JSON format: exercism/problem-specifications#1536 - 2.0.0 to 2.0.1, capitalize Scrabble: exercism/problem-specifications#1575
Such a long test is really a comment, not a description to be used as a test identifier.
Make it so, per #1473