Skip to content

Minor updates - and remove body printing#66

Open
seanhess wants to merge 1 commit intomarkandrus:masterfrom
seanhess:master
Open

Minor updates - and remove body printing#66
seanhess wants to merge 1 commit intomarkandrus:masterfrom
seanhess:master

Conversation

@seanhess
Copy link
Copy Markdown

@seanhess seanhess commented Feb 2, 2019

I updated containers to be compatible with 0.5 and 0.6, then removed the Data.Monoid imports (no longer neccesary).

I removed the print statement in runRequest', because it makes the library unusable (my programs log to stdout, so this means user information can become leaked to the logs.) I'd recommend leaving it out until you make it configurable. (Debug mode?)

@markandrus
Copy link
Copy Markdown
Owner

Thanks @seanhess,

It looks like removing the Data.Monoid import broke some modules on older GHCs. I think we need to add it back. Otherwise, thanks for bumping containers and thanks for removing the print statement (definitely my mistake to have included it!).

let body = responseBody response
body' <- LBS.fromChunks <$> brConsume body
print body'
-- print body'
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK to just delete this, IMO.

Comment thread twilio.cabal
binary >=0.7,
bytestring ==0.10.*,
containers ==0.5.*,
containers >= 0.5.0 && <0.7.0,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can just nix the upper bound completely?

Comment thread src/Twilio/APIKey.hs
import Control.Monad
import Control.Monad.Catch
import Data.Aeson
import Data.Monoid
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some errors like

src/Twilio/Types/SID/TH.hs:59:6:
    Not in scope: ‘<>’
    Perhaps you meant one of these:
      ‘<$>’ (imported from Prelude), ‘*>’ (imported from Prelude),
      ‘<$’ (imported from Prelude)

So I think we need to add these imports back.

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.

2 participants