Skip to content

leap: update example_gen.go & test cases #444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 17, 2017

Conversation

robphoenix
Copy link
Contributor

the filepath needed used by example_gen.go was changed in #224
the test cases were changed in this commit:
exercism/problem-specifications#463

@@ -15,7 +16,8 @@ func main() {
log.Fatal(err)
}
var j js
if err := gen.Gen("leap.json", &j, t); err != nil {
f := path.Join("exercises", "leap", "canonical-data.json")
Copy link
Member

@petertseng petertseng Jan 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also reference #357 in the commit message, due to this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the effect of making this change across every single generator - we will be typing path.Join("exercises", whateverSlug, "canonical-data.json") a lot. Can/should this line be moved into the Gen function, and we just pass leap into Gen?

I won't oppose this change for this PR, but I'd advise it if working on more generators.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally missed this issue, I stumbled upon this through trying to implement a test generator in another exercise.
I was thinking the same thing, I'll look into cancelling my other PR and updating the Gen function. Leave this one unmerged and hopefully I can update it along with the others.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably don't have to cancel it - all the generators will still need to update to pass in only the slug rather than the filename (leap rather than leap.json), can just repurpose it to do the Gen update in that PR. Whichever is easier for you if you're going to work on it though.

the filepath needed used by example_gen.go was changed in exercism#224
the test cases were changed in this commit:
exercism/problem-specifications#463
@robphoenix robphoenix merged commit a89f0f7 into exercism:master Jan 17, 2017
@robphoenix robphoenix deleted the leap/update-tests branch January 17, 2017 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants