-
Notifications
You must be signed in to change notification settings - Fork 97
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 compatibility with ghc-7.10 #189
Conversation
This PR depends on: (under review) haskell-distributed/distributed-static#4 |
I have splitted commits into small one and verified build on 7.8 and 7.10 |
Building with ghc-7.4 is failing for some reason. |
@@ -137,7 +137,7 @@ import GHC.Generics | |||
newtype NodeId = NodeId { nodeAddress :: NT.EndPointAddress } | |||
deriving (Eq, Ord, Typeable, Data, Generic) | |||
instance Binary NodeId where | |||
instance NFData NodeId | |||
instance NFData NodeId where rnf (NodeId a) = rnf a `seq` () |
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.
We should depend on deepseq>=1.4 and drop the where rnf (NodeId a) = rnf a seq ()
.
http://hackage.haskell.org/package/deepseq-1.4.1.1/docs/Control-DeepSeq.html#t:NFData
Unless I'm misunderstanding the point of this change?
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.
There are 2 possibilities, use generic deriving on >deepseq-1.4
and write instances manually on previous versions, or write instances manually everywhere. I find latter approach more straightforward as former implies latter under if-condition.
I fixed NFData instance for LazyByteString argument. |
Can't build this right now and travis is failing. Can anyone confirm it had been fixed for 7.4? |
I see "all is well" in travis report: https://travis-ci.org/haskell-distributed/distributed-process/builds/57053812 @hyperthunk you can run rebuild in top right corner (near settings). |
Yes will do, I've been away for a month so I'm just catching up again. |
Compiling failed on 7.10.1
|
@isaiah have you used patched version of |
@qnikst Yes, install |
This one works good for several people on the zurihack, and passes tests, so I'm going to merge this one. |
No description provided.