Skip to content

leap: update example_gen.go & test cases#444

Merged
robphoenix merged 1 commit intoexercism:masterfrom
robphoenix:leap/update-tests
Jan 17, 2017
Merged

leap: update example_gen.go & test cases#444
robphoenix merged 1 commit intoexercism:masterfrom
robphoenix:leap/update-tests

Conversation

@robphoenix
Copy link
Copy Markdown
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

Comment thread exercises/leap/example_gen.go Outdated
}
var j js
if err := gen.Gen("leap.json", &j, t); err != nil {
f := path.Join("exercises", "leap", "canonical-data.json")
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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