Repository navigation
Fix build with crypton; revive doctests; resolve #2 - #9
Merged
Merged
Conversation
Since aeson 2.0.0.0 (~2021), its KeyMap is indexed by separate type Key -- no longer by Text. This commit then fixes typecheck error in tests: > tests/src/Web/JWTInteropTests.hs:59:38: error: [GHC-83865] > • Couldn't match expected type ‘Key’ with actual type ‘T.Text’ > • In the first argument of ‘key’, namely ‘key'’ > In the second argument of ‘(^?)’, namely ‘key key'’ > In the expression: toJSON claims' ^? key key' > | > 59 | let json = toJSON claims' ^? key key' > | ^^^^ And correspondignly, the test can no longer build with aeson-v1; thus the version requirement bump.
Aeson 2 has been released 5 years ago. The MIN_VERSION_aeson CPP macros are presenting extra hurdles to doctests.
This reverts commit 4390e2e. `cabal repl --with-compiler=doctest` is currently _the_ recommended approach to invoke doctest from CI. Other approaches hit various complications with dependencies (e.g. doctest of the main JWT module requires importable crypton and others), or are not yet mature enough (the `cabal doctest` command requires fairly fresh cabal-install 3.12). With this, `cabal test test:doctests` fails only 2 examples, not all 8.
Kudos to @ocheron for handling the new package release 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Brian @puffnfresh !
I was looking into what'd be needed to get "the" haskell jwt library 😃 back into Stackage LTS snapshots. Ended up with this maintenance patch series. 👀 Please review.
Apologies for piling up several things together; one thing led to another... The overall PR came out not that large, hoping won't be a big hassle to approve for merging.
On top of individual commit messages, some higher-level bullet points for context:
jwtin 994f351b within PR Removex509*packages commercialhaskell/stackage#7431 — due to abandonment of x509-* family of packages, which included some ofjwtdependees. cc @mihaimaruseac.cryptostoreitself was having issues with+use_crypton; it wasn't bulding.This means
jwtis finally in good position to adopt+use_crypton, as the blocker hinted at in #8 is now resolved.I took this deep-dive as opportunity to do a check-up on the doctests as well — which led to reverting 4390e2e and a few other minor changes. In particular, #2 becomes resolved by accident. Doctests do run and pass. ✅
One of the minor changes was dropping aeson <2 — its v2 is fairly mature by now, released 5 years ago, and has had good adoption in many packages across the ecosystem. LMK if you'd like to support aeson v1 anyway; I'll try to see what can be done to force CPP + doctests into good neighborship.
Resolves #2.
Supersedes #3.
Continues #5 and #8.