search

ongres / scram

SCRAM (RFC 5802) Java implementation
BSD 2-Clause "Simplified" License
26 stars 11 forks source link

Add module path support #5

Closed alexanderkjall closed 3 years ago

alexanderkjall commented 5 years ago

Upgrade to java 9 and add module information.

I realize this isn't a very likely pull request to get merged, as it's a pretty big thing to upgrade the requirements to java 9. But I thought that since I opened bug #3 I should also go ahead and make a pull request for it.

alexanderkjall commented 5 years ago

I reworked the patch so that it only adds a manifest line instead of upgrading the java version to 9, from a suggestion from @fwgreen

davecramer commented 5 years ago

@ahachete thoughts on this ? Any reason not to do it ?

ahachete commented 5 years ago

As much as I like the module system in Java 9, I wonder how many potential users of the library may not be happy for jumping on this minimum requirement. Other than another project is depending on the module system, what would be a good reason to do this? I'd do it Java9+ if done from the beginning, but as of today, I'd keep the minimum on Java8.

Unless, there would be a nice proposal to support both Java8 and Java9+ at the same time. Thoughts?

fwgreen commented 5 years ago

Adding the Automatic-Module-Name entry to MANIFEST.MF does not break Java 8 compatibility, in fact it was added to the jar file spec to ease migration. Here is a pretty good write-up on the subject: https://blog.joda.org/2017/05/java-se-9-jpms-automatic-modules.html

alexanderkjall commented 5 years ago

@ahachete sorry that I wasn't clearer on that the patch didn't change the java version. If you have any feedback on it, then i'm happy to do some additional work on the patch in order to get it merged.

ahachete commented 3 years ago

Hi @alexanderkjall

In preparation for a new release, I'm ready to merge this PR. Would you mind rebasing it on top of https://github.com/ongres/scram/tree/1.0.0-beta.3 (which contains some related changes) and as a single commit if it makes sense?

Thank you very much.

alexanderkjall commented 3 years ago

@ahachete sure, let me do that.

alexanderkjall commented 3 years ago

@ahachete I think that it's updated now, but to be honest it's a long time since I wrote this code and I only reran the build scripts to check that it builds without problem. 'caveat emptor' I guess :)

jorsol commented 3 years ago

@alexanderkjall could you please send the Automatic-Module-Name change to https://gitlab.com/ongresinc/scram/?