-
Notifications
You must be signed in to change notification settings - Fork 58
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
Make compile cleanly with stack --pedantic #82
Conversation
@@ -62,17 +62,17 @@ makePrisms ''Referenced | |||
|
|||
_SwaggerItemsArray :: Review (SwaggerItems 'SwaggerKindSchema) [Referenced Schema] | |||
_SwaggerItemsArray | |||
= prism (\x -> SwaggerItemsArray x) $ \x -> case x of | |||
= unto (\x -> SwaggerItemsArray x) {- $ \x -> case x of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fizruk I wasn't sure, if this and _SwaggerItemsObject
are lawful Prism'
, so changed to unto
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What made you suspicious? I don't see any violation (not looking very hard).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the type is Review
and not Prism'
, so I fixed the implementation (term) not the spec (type) :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So should I change type to Prism'
and try to "prove" the laws in tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I didn't realise I had Review
type! I wonder why I abandoned Prism
... perhaps it did not compile with some old GHC. Thanks anyway!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't this you should bother, to be honest :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, cannot parse the previous, is this = think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, don't waste your time on this.
cc620bf
to
220e66a
Compare
, base-compat >=0.6.0 && <0.10 | ||
, aeson | ||
, base-compat >=0.9.1 && <0.10 | ||
, aeson >=0.11.2.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed for ToJSON Day
, only aeson-0.10
introduced it, but skipping directly to 0.11 shouldn't be a problem for any user. EDIT: there was typo: should
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, shouldn't.
220e66a
to
68e95a6
Compare
No description provided.