Skip to content
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

Add support for Thoas (a yet another JSON library) #126

Merged
merged 3 commits into from Sep 1, 2022

Conversation

michaelklishin
Copy link
Contributor

@michaelklishin michaelklishin commented Jul 29, 2022

@ericmj sorry to ping you directly but this turned out to be an unexpected blocker for Thoas adoption in RabbitMQ :) If you agree to accept this PR, we'd appreciate a new tag we can depend on.

@michaelklishin
Copy link
Contributor Author

The failures are due to git clone/TCP connection timeouts, GitHub is having a rough day it seems. I run into the same issue locally.

@michaelklishin
Copy link
Contributor Author

@ericmj is this something you'd be interested in accepting or should Team RabbitMQ use our own fork? Thank you 🙏

@ericmj
Copy link
Collaborator

ericmj commented Aug 2, 2022

I want to include this but it's blocked by CI failing due to the no longer supported git:// protocol: https://github.com/talklittle/erlang-libdecaf/blob/otp-23-remove-erl-interface/c_src/Makefile#L107.

I don't really have the time or knowledge of this library to fully maintain it. I can only merge PRs that I confirm to work by CI passing and make new releases until @potatosalad comes back or someone else is willing to take over and do proper maintainership. If you want this merged I unfortunately must ask you or another contributor to first fix CI.

We may be able to change the libdecaf dependency back to https://github.com/potatosalad/erlang-libdecaf but it needs to be updated or forked to use a supported git protocol.

@potatosalad
Copy link
Owner

I can take a look at this soon.

@michaelklishin
Copy link
Contributor Author

@potatosalad thank you. If you'd like someone to help out with this library, I'd be happy to. I am not a crypto expert but I'm sure there are issues that either won't require much knowledge (like this one) or, given enough motivation, new maintainers can meaningfully help with.

@michaelklishin
Copy link
Contributor Author

It's been more than two weeks, any updates on this? It's a pretty low risk change :)

@michaelklishin
Copy link
Contributor Author

It's been close to a month now, @potatosalad should I assume that JOSE maintainers are not interested in this PR and RabbitMQ should run a fork of JOSE from now on?

@michaelklishin
Copy link
Contributor Author

Rebased on top of main

@ericmj
Copy link
Collaborator

ericmj commented Aug 29, 2022

We can merge this if you also fix CI. That can be done in a separate PR or in this PR. Feel free to fork libdecaf if you need to.

@potatosalad
Copy link
Owner

@ericmj I got libdecaf 2.1.0 released yesterday which should fix the CI issues there, currently working on getting libsodium functional again on modern OTP so that this project's CI can work again.

@ericmj
Copy link
Collaborator

ericmj commented Aug 29, 2022

@potatosalad Thank you very much. Greatly appreciated!

@potatosalad potatosalad merged commit 58798c8 into potatosalad:main Sep 1, 2022
@potatosalad potatosalad added this to the jose 1.12.0 milestone Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants