Conversation
| mkdir -p ${temp_dir} | ||
| mkdir -p "${temp_dir}" | ||
| # Copy relevant files to the temp directory (replace solution with example) | ||
| solution_file_name="$(jq -r '.files.solution[0]' $1/.meta/config.json)" |
There was a problem hiding this comment.
Why are you not using the config.json? That's more reliable.
There was a problem hiding this comment.
For consistency with the preexisting test_runner.gd that this script indirectly calls:
solution_script_path = gdscript_path + ".gd"
test_suite_script_path = gdscript_path + "_test.gd"Since that's already being assumed there, this seemed the most straightforward way to remove jq from this spot. As it currently is (and already had been before this PR), following config.json here but not in the test runner would make the test runner fail if an exercise deviated from this convention.
I see that the behavior in test_runner.gd is implied by the spec, because test_runner.gd normally wouldn't have access to config.json (especially if a tmp dir is used that doesn't contain it).
For consistency with other tracks, we could ditch bash entirely in favor of GDScript for verify-exercises — then we can have JSON parsing for free here. (That's what the Python track seems to do.)
There was a problem hiding this comment.
Assumptions are precursors to bugs ;) But if that assumption is already baked in... 🤷
For consistency with other tracks, we could ditch bash entirely in favor of GDScript for verify-exercises — then we can have JSON parsing for free here. (That's what the Python track seems to do.)
If that's not too much work, it would be a nice improvement.
There was a problem hiding this comment.
@meatball133 I can do this but are you OK with that, or was there some reason verify-exercises needed to be a bash script?
There was a problem hiding this comment.
There was a problem hiding this comment.
For consistency with other tracks, we could ditch bash entirely in favor of GDScript for verify-exercises — then we can have JSON parsing for free here. (That's what the Python track seems to do.)
If that's not too much work, it would be a nice improvement.
OK, it's converted ✔
There was a problem hiding this comment.
Still depends on exercism/gdscript-test-runner#54 to pass though.
3 participants
https://forum.exercism.org/t/create-new-track-for-gdscript/3955/209
Goes with exercism/gdscript-test-runner#54, which needs to be merged in order for the CI check here to pass (and both need merging for any other PR's CI checks in this repo e.g. #101's recent failure, to pass).