-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minimal Update to JWTKit 5 #68
Minimal Update to JWTKit 5 #68
Conversation
Co-Authored-By: Tim Condon <[email protected]> Co-Authored-By: Paul Toffoloni <[email protected]>
If the main concern is changing the major version dependency from 4 to 5, vapor/jwt-kit#221 will help add additional support that this library in particular can use to remain backwards compatible with version 4 while also supporting 5. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dimitribouniol Thank you for this submission, was waiting for a release with breaking changes to put this into, this looks good!
Would finishing the compatibility update for jwt-kitv4 help get this in sooner then? This way it won’t require a release with breaking changes. If so, I can prioritize getting that finished this week. |
Looks like this is not possible on its own, it requires Swift 6 |
I’ll get that compatibility layer in then - this way we can keep the version locked to the latest v4 patch in Package.swift, and we can have a Package-swift6.0.swift that uses jwt-kit v4-5. I think that should cover all our bases? (this code in the PR will incidentally stay the same since the compatibility layer adds the new v5 API surface to the v4 version branch) |
@dimitribouniol We are upgrading the library to Swift 6 as part of this breaking change, and after doing so tests are passing |
That works too haha |
While I have you re: breaking changes, this would also be a good one to consider addressing: #52 |
It’s in my local branch already, just have to push it out |
Sweet, thanks for the updates! |
Based on #14, here is the minimal set of changes to move this over to JWTKit 5. As this library is likely blocking adoption for many upstream projects, I'm hoping this can accelerate the process before more full-fledged support can be added from #14.