-
Notifications
You must be signed in to change notification settings - Fork 3
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
enable to use PEM feature without nanojson nor tomitribe-util #6
base: master
Are you sure you want to change the base?
Conversation
…ee being the key there)
Hey @rmannibucau checkout the new groupId and artifactId, these don't have dependencies on nanojson or tomitribe-util |
Any hope we get it without cheating with the shade + get the test in to ensure it is a feature of the deliverable? |
Use of the shade plugin was intentional as to avoid forking of the related code, the JSON parser in particular. Note this PR does not address the JSON parser. On the test are you referring to something in the PR or something yet to be written? |
This pr guarantees pem supports works without nanojson and tomitribe-util (and the shade) and harness it by running pem tests (disabling jwk related ones). Side note: ideally I think using a spi - hopefully not hardcoded to enable to use cdi or a serviceloader in standalone - and make indep modules to solve the dep more elegantly and explicitly would be the best but it sounded too intrusive so I just ensured my cases were running without deps and harnessed it by test execution on the ci. |
What is the issue you're facing? |
|
This PR doesn't do those things. The shaded libraries are still there in the final jar, including tomitribe-util and nanojson. This PR creates the same shaded jar, but is 4k larger due to the new code. It looks like tomitribe-util could be removed with this PR, but the dependency on nanojson wasn't addressed. As mentioned the shade work was done very recently and I did consider copying the source code, but opted not to as it then tends to never get updated. I'd like to be able to easily get updates on the JSON parser when they're released without having to copy/paste them. The shading was setup to only bring in the classes needed so there isn't any useless code from unused classes. I'd prefer to leave it as-is for now if that's ok. |
It is David, only it does not do is to remove/classify the shade but rest is done. Only thing needed is to merge to enable github workflow. Check out the yaml. |
Goal to use it "dep free" is to lighten integrations but also make it more shebang/jshell friendly in scripts (less deps = less setup).