move to stack with lts-8.19 #42
Conversation
|
Thanks, Chad. I'll take a look when I return from vacation.
…On Thu, Jun 22, 2017 at 8:20 PM Chad Selph ***@***.***> wrote:
I don't actually know what I'm doing, but I got this to compile.
I can't run the tests because they only work on your account.
------------------------------
You can view, comment on, or merge this pull request online at:
#42
Commit Summary
- move to stack with lts-8.19
File Changes
- *M* .gitignore
<https://github.com/markandrus/twilio-haskell/pull/42/files#diff-0>
(1)
- *M* src/Twilio/Types.hs
<https://github.com/markandrus/twilio-haskell/pull/42/files#diff-1>
(19)
- *A* stack.yaml
<https://github.com/markandrus/twilio-haskell/pull/42/files#diff-2>
(66)
- *M* twilio.cabal
<https://github.com/markandrus/twilio-haskell/pull/42/files#diff-3>
(56)
Patch Links:
- https://github.com/markandrus/twilio-haskell/pull/42.patch
- https://github.com/markandrus/twilio-haskell/pull/42.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#42>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAV68g2P3SWzLOtm4rcHxGhlqbNphjvXks5sGrBxgaJpZM4OCqZB>
.
|
src/Twilio/Types.hs
Outdated
| throwForNon204 _ resp = | ||
| case responseStatus resp of | ||
| Status 204 _ -> return () | ||
| status -> do |
There was a problem hiding this comment.
Since status is unused, why not
Status 204 _ -> return ()
_ -> doAlso, we may want to prefer pure to return, but it's a minor thing. Better to catch with some kind of style-check in the future...
There was a problem hiding this comment.
I didn't know the difference between pure and return. I actually had pure first, I can change it back.
twilio.cabal
Outdated
| http-client-tls, | ||
| network-uri, | ||
| text, | ||
| transformers, |
There was a problem hiding this comment.
I don't know much about Stack, but in order for this package to continue working with vanilla Cabal/Hackage, don't we need to keep some known version bounds in the .cabal file?
There was a problem hiding this comment.
yeah I don't know either.
my understanding is stack will generate the .cabal file with the specific versions (found in the build-plan: lts-8.19)
|
Thanks, @chadselph. I had one comment and one question, namely: will this continue working with vanilla Cabal/Hackage?
Right, Travis keeps the credentials private on PRs from forks. Any thoughts on how to improve the tests? #31 is one path forward for unit tests, and then integration tests (requiring credentials) could perhaps be optional. |
|
#31 seems good, I was kind of surprised it wasn't already working this way. But is it possible to make that change backwards compatibly? |
|
Thanks @chadselph for making these updates. I was just about to make them myself, but was glad to see someone had beat me to it. I opened a PR on your fork to re-add the package bounds in the cabal file. |
Add bounds for packages
|
Thanks @chadselph @Narrative I'll try this out now. I think we'll probably want to update the |
I don't actually know what I'm doing, but I got this to compile.
I can't run the tests because they only work on your account.