add Web.Scotty.Cookie #293
Conversation
|
this LGTM Can you add tests? |
|
I don't understand why is CI failing ; I've built thir PR successfully with GHC 9.2.7 |
|
I was afraid I would have to pull webdriver out but we have hspec-wai :) |
|
Tests added @chessai ^^ LMK |
Web/Scotty/Cookie.hs
Outdated
| -- ==== Cookie Configuration | ||
| -- | ||
| -- Cookies have several configuration options; a brief summary of each option is given below. For more information, see <http://tools.ietf.org/html/rfc6265#section-4.1.2 RFC 6265> or <https://en.wikipedia.org/wiki/HTTP_cookie#Cookie_attributes Wikipedia>. | ||
| data SetCookie = SetCookie |
There was a problem hiding this comment.
What's the reason behind duplicating the cookie package's definition?
There was a problem hiding this comment.
mainly to decouple them, but it's also unlikely that the definition of a web cookie will change in the future, so I will revert to importing 'cookie' rather than vendoring it in.
There was a problem hiding this comment.
Great, I think that's better
|
@fumieval we now import 'cookie' and re-export some of its definitions. |
Web/Scotty/Cookie.hs
Outdated
| -- ** parsing and rendering | ||
| , parseSetCookie | ||
| , renderCookies | ||
| , renderCookiesText |
There was a problem hiding this comment.
Perhaps "parsing and rendering" section is not necessary
|
Hi @fumieval , could you merge this if there are no more questions? |
|
Just triggered CI, if it passes I'll merge |
|
CI failures are most puzzling. I did introduce two new dependencies in this PR (cookie and time), and perhaps the lower bounds need to be relaxed (even though neither of them is mentioned in the CI failures). I've only tested this PR with stack, I don't use cabal |
scotty.cabal
Outdated
| @@ -86,6 +88,7 @@ Library | |||
| network >= 2.6.0.2 && < 3.2, | |||
| regex-compat >= 0.95.1 && < 0.96, | |||
| text >= 0.11.3.1 && < 2.1, | |||
| time >= 1.11, | |||
There was a problem hiding this comment.
time >= 1.11 seems too strict. How about >= 1.8?
There was a problem hiding this comment.
done! All tests are green on my end, try running CI again?
|
@fumieval I've gotten rid of some unnecessary imports in one of the example binaries. Hopefully now CI will be green at least for the recent GHCs. I really don't know why don't older GHCs work |
|
The CI failure for 8.6.5 seems to be due to a change in a recent version of 'wai-extra' which now imports 'crypton-x509' instead of 'x509' https://github.com/scotty-web/scotty/actions/runs/5282948607/jobs/9564762917?pr=293#step:17:61 The failures on GHC <= 8.0.2 are still mysterious to me |
|
I think it's fine to drop support for GHC < 8.10 (there's #289 too). Thoughts? @chessai @RyanGlScott |
|
Can I do something to get this PR across the finish line? |
Fine by me |
addresses #253
cc @chessai ^^