Skip to content
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

series: rewrite to be flexible and improve stub. #427

Merged
merged 2 commits into from
Nov 8, 2016
Merged

series: rewrite to be flexible and improve stub. #427

merged 2 commits into from
Nov 8, 2016

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Nov 8, 2016

  • Change the test suite to accept multiple signatures.
  • Add HINTS.md explaining how to work the exercise.
  • Add a example solution using ByteStrings and Sequences.
  • Add type signature to the stub solution.
  • Add variables to the stub implementation.

These change also have the effect of allowing to run the test suite with the stub solution.

Related to #421.

@petertseng
Copy link
Member

These change also have the effect of allowing to run the test suite with the stub solution.

Oh, was it unable to before...?

Oh, indeed it was.

    • Ambiguous type variable ‘t0’ arising from a use of ‘shouldBe’
      prevents the constraint ‘(Eq t0)’ from being solved.

I see.

@petertseng
Copy link
Member

I agree with every change I see here. Can I take a moment to be a stickler about separate commits?

I think one commit should be "improve stub", and the other commit everything else - everything else seems related, but "improve stub" seemed like the odd duck out in the big commit and seemed like it could go in its own commit. True, that would make the commit very small, but it seemed to make sense to me.

Unless you think the two I mentioned are inextricably linked, in which case sure, just tell me why and then merge it.

- Add type signature.
- Add variables to the stub implementation.

These change also have the effect of allowing to run the
test suite with the stub solution.
- Change the test suite to accept multiple signatures.
- Add HINTS.md explaining how to work the exercise.
- Add a example solution using ByteStrings and Sequences.
@rbasso
Copy link
Contributor Author

rbasso commented Nov 8, 2016

You are right, they belong to separated commits. Fixed! 😄

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

Awesome. I'll merge when Travis CI finishes. Feel free to beat me to it if I look away from the computer for too long.

Edit: Just to show that we can edit these.

@petertseng
Copy link
Member

petertseng commented Nov 8, 2016

Oh good, I can edit approve/request changes comments now (we were not able to before, which is why I always made it a separate comment)

@petertseng petertseng merged commit c1d21c3 into exercism:master Nov 8, 2016
@rbasso rbasso deleted the series-make-stub-build-test branch November 8, 2016 18:08
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