Skip to content

Conversation

@paf31
Copy link
Contributor

@paf31 paf31 commented Jan 30, 2016

This a slightly different take on @hdgarrood's branch, using Data.Op for Option and dropping key. As we add combinators to Data.Functor.Contravariant.Divisible, this approach will become more useful, I think.

@ethul
Copy link
Collaborator

ethul commented Jan 30, 2016

Thanks! Looks good. I will dig more into this on the weekend.

On Friday, 29 January 2016, Phil Freeman [email protected] wrote:

This a slightly different take on @hdgarrood
https://github.com/hdgarrood's branch, using Data.Op for Option and
dropping key. As we add combinators to
Data.Functor.Contravariant.Divisible, this approach will become more

useful, I think.

You can view, comment on, or merge this pull request online at:

#13
Commit Summary

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#13.

@ethul
Copy link
Collaborator

ethul commented Jan 31, 2016

Sorry for the delay, but I think the PR looks great. Before merging, is this intended only for purescript 0.8? I am wondering if we should update the exports and corresponding imports for 0.7 compatibility? Or we could release this along with 0.8. That works for me too.

Also, do you think this module should be moved to core or contrib once this PR is merged? I am open to either.

@garyb
Copy link
Member

garyb commented Jan 31, 2016

Probably stick to 0.7 for now, all the core libraries are going to be updated and bumped after 0.8 is released, so it will need updating again anyway.

ethul added a commit that referenced this pull request Feb 2, 2016
@ethul ethul merged commit 244c9c9 into purescript-contrib:master Feb 2, 2016
@ethul
Copy link
Collaborator

ethul commented Feb 2, 2016

Before publishing to pursuit is this good to move into core?

@hdgarrood
Copy link
Contributor

🎉

@paf31 paf31 deleted the feature/use-op branch February 2, 2016 16:50
@paf31
Copy link
Contributor Author

paf31 commented Feb 2, 2016

@ethul If you're okay with it, I think it would be a good addition. @garyb Anything else we need?

@garyb
Copy link
Member

garyb commented Feb 2, 2016

Nope, 👍 for moving over. Core or contrib?

@paf31
Copy link
Contributor Author

paf31 commented Feb 2, 2016

Perhaps contrib for now, until we're confident the new API is the right one, with a view to moving it to core soonish.

@garyb
Copy link
Member

garyb commented Feb 2, 2016

Sounds sensible to me.

@paf31
Copy link
Contributor Author

paf31 commented Feb 2, 2016

@ethul You should be able to move the repo to contrib now, if you like.

@ethul
Copy link
Collaborator

ethul commented Feb 2, 2016

Thanks! The repo has been moved over

@paf31
Copy link
Contributor Author

paf31 commented Feb 2, 2016

Thanks! I've set up a new team.

@ethul
Copy link
Collaborator

ethul commented Feb 2, 2016

Great, thank you!

On Tuesday, 2 February 2016, Phil Freeman [email protected] wrote:

Thanks! I've set up a new team.


Reply to this email directly or view it on GitHub
#13 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants