Skip to content

Arb instances #3

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 2 commits into from
Oct 1, 2014
Merged

Arb instances #3

merged 2 commits into from
Oct 1, 2014

Conversation

jdegoes
Copy link
Contributor

@jdegoes jdegoes commented Oct 1, 2014

No description provided.

@jdegoes
Copy link
Contributor Author

jdegoes commented Oct 1, 2014

Review by @paf31

paf31 added a commit that referenced this pull request Oct 1, 2014
@paf31 paf31 merged commit 57e5071 into purescript:master Oct 1, 2014
@paf31
Copy link
Contributor

paf31 commented Oct 1, 2014

Cool, thanks. I had to think a bit about what we want our best practices to be about arb instances, but I think this is good - core instances in Test.QuickCheck, other things can depend on quickcheck. It seems a little bit of a shame for lists to have a dependency on quickcheck, but it's not the end of the world.

@jdegoes
Copy link
Contributor Author

jdegoes commented Oct 1, 2014

I agree. I think ultimately every library that exports data types should have a dependency on quickcheck. The alternative orphan-less solution is for quickcheck to have a dependency on every library in the world, which is probably the greater of two evils. :)

@jdegoes jdegoes deleted the arb-instances branch October 1, 2014 21:42
@jdegoes
Copy link
Contributor Author

jdegoes commented Oct 1, 2014

Btw I am not tagging these commits, which is how I gather bower decides what the version should be. I assume merger should tag the commits, or should the submitter of the PR tag them with what they think should be the correct version number?

@paf31
Copy link
Contributor

paf31 commented Oct 1, 2014

I need to make a release, which I'll do now. Then bower will pick up the new version.

@paf31
Copy link
Contributor

paf31 commented Oct 1, 2014

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