brendanhay / amazonka

A comprehensive Amazon Web Services SDK for Haskell.
https://amazonka.brendanhay.nz
Other
599 stars 227 forks source link

mtl-2.3 compatibility #779

Closed ysangkok closed 2 years ago

ysangkok commented 2 years ago

Before this PR, compilation on mtl-2.3 fails with

src/Amazonka/Error.hs:98:51: error:
    • No instance for (Alternative (Either String))
        arising from a use of ‘<|>’
    • In the third argument of ‘either’, namely
        ‘(h .# hAMZRequestId <|> h .# hAMZNRequestId)’
      In the expression:
        either
          (const Nothing) Just (h .# hAMZRequestId <|> h .# hAMZNRequestId)
      In an equation for ‘getRequestId’:
          getRequestId h
            = either
                (const Nothing) Just (h .# hAMZRequestId <|> h .# hAMZNRequestId)
   |
98 |   either (const Nothing) Just (h .# hAMZRequestId <|> h .# hAMZNRequestId)

The instance Alternative (Either String) makes little sense to me since I don't know how the instance can manufacture an empty value for the Alternative instance.

So in this PR, I try to avoid this instance.

This is untested, I'd appreciate advice on how best to test this.

ysangkok commented 2 years ago

Testing is going well, cabal test --constraint='mtl>=2.3' is passing with no issues. And I can build the examples.

endgame commented 2 years ago

@ysangkok is there anything else you want to test before I merge?

ysangkok commented 2 years ago

@endgame I was hoping that somebody would get the chance to test in a 'staging' first. Sadly at my work we don't have that infrastructure for the project we use Amazonka for, so I would have to test it in production, which is kinda dangerous. I think it looks fairly safe since the worst case seems to be that it passes a Nothing to serviceError, in place of making all of go a Nothing, which doesn't sound too dangerous, but I haven't looked into it.