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

Add ParamSchema & Schema for Numeric.Natural #123

Merged
merged 4 commits into from
Sep 21, 2017

Conversation

gkleen
Copy link
Contributor

@gkleen gkleen commented Sep 20, 2017

Seems reasonable to have a Schema for Numeric.Natural; it's in base and has aeson-support.

@phadej
Copy link
Collaborator

phadej commented Sep 20, 2017

you need to add

  if !impl(ghc >= 7.10)
    build-depends:
      nats               >=1.1.1    && <1.2

to make GHC-7.8.4 happy.

@gkleen
Copy link
Contributor Author

gkleen commented Sep 20, 2017

Okay, I don't have a testing environment for anything but the current ghc version set up, so let's just see if travis is happy :-)

I also don't think we need to restrict the version of nats to anything other than < 2; all we really need is that a type called Natural exists

@fizruk
Copy link
Member

fizruk commented Sep 20, 2017

@pngwjpgh thanks for your contributions!

@phadej feel free to merge this and #121 when you're happy with dependency constraints.
I can make a release shortly after.

@phadej
Copy link
Collaborator

phadej commented Sep 20, 2017

FWIW, I'd really like to have both upper and lower bounds. It's unluckily nats will get major updated, but if it will, it most likely be broken in a bad way.

@gkleen
Copy link
Contributor Author

gkleen commented Sep 20, 2017

Would you be okay with >=0.1 && <2 then?

Version 0.1 is the first one and introduces Numeric.Natural.Natural.
By semantic versioning dropping the name Numeric.Natural.Natural for the type would lead to a version 2.* being an incompatible api change.

Afaik @ekmett has always been good with adhering to semantic versioning.

@phadej
Copy link
Collaborator

phadej commented Sep 20, 2017

I insist on >=1.1.1 && <1.2. There is no need to support older versions. If we haven't ever tested with 0.1, then we shouldn't claim we support it. Travis says on GHC-7.8.4 job

Loading package nats-1.1.1 ... linking ... done.

Hackage uses PVP where two first digits act as major, third as a minor and fourth as a patch. So in this case nats-1.2 might introduce breaking change (unluckily, but still).

@phadej
Copy link
Collaborator

phadej commented Sep 20, 2017

also rebase needed now.

@gkleen
Copy link
Contributor Author

gkleen commented Sep 20, 2017

Valid points. Thanks for linking me to PVP, I didn't know about that :-D

@ekmett
Copy link

ekmett commented Sep 21, 2017

Just for the record, I follow the PvP as closely as I can. In addition I reserve the first digit for major overhauls that break my own packages. (This can make my package bounds look a bit like sem-ver.) Restricting off the second digit as @phadej notes is the correct option per the PVP.

@phadej phadej merged commit 5ec8bd7 into GetShopTV:master Sep 21, 2017
@gkleen gkleen deleted the feature/Natural branch September 21, 2017 17:25
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.

4 participants