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

Use a multimodule maven organisation #37

Merged
merged 1 commit into from
Mar 25, 2018

Conversation

victornoel
Copy link
Member

  • Change groupId to org.pac4j.jax-rs
  • Users should depend on jersey-pac4j or resteasy-pac4j
  • Add saveInSession parameter for callback endpoints

This simplifies and re-enforce the correctness of the testing and will ease the support of jersey 2.26 and other implementations (see #36 #30).

@pierre-l if you have some feedback on that? I will then do the same for branch 2.x and add a legacy dependency org.pac4j.jax-rs-pac4j for backward compatibility.

@leleuj once this is merged, dropwizard-pac4j will only depends on jersey-pac4j.

@victornoel
Copy link
Member Author

@leleuj open question: should jersey-pac4j and resteasy-pac4j be under org.pac4j.jax-rs (like the rest of the submodules) or directly under org.pac4j (like jax-rs-pac4j and to keep things clear for users)?

@pierre-l
Copy link
Contributor

@victornoel I'm not sure about the "test-tools" name I initially suggested, what do you think about "testing-tools" or just "testing"? Other than that, it seems pretty good to me. The part I liked the less in my commits for #36 won't be needed, I'm happy about this. I'll try to integrate this in our own project tomorrow in order to see if anything breaks. I'll let you know. Thanks!

@leleuj
Copy link
Member

leleuj commented Mar 21, 2018

About the groupId, I would stick to org.pac4j for consistency with other pac4j libraries (in Play, we have two different libraries, one for Java and one for Scala, but both have the same groupId).
That said, it's not a major issue.

@victornoel
Copy link
Member Author

About the groupId, I would stick to org.pac4j for consistency with other pac4j libraries (in Play, we have two different libraries, one for Java and one for Scala, but both have the same groupId).
That said, it's not a major issue.

Ok, I will keep org.pac4j.jax-rs for the modules not meant to be used by users and org.pac4j for the dependencies the users require!

I will also rename the test-tools module to testing, it makes sense.

 - Users should depend on jersey-pac4j or resteasy-pac4j
 - jax-rs-pac4j has been removed
 - Add saveInSession parameter for callback endpoints
@victornoel
Copy link
Member Author

I updated the PR, I will merge it later tonight except if you have other feedbacks :)

@pierre-l
Copy link
Contributor

Nothing to report here.

@victornoel victornoel merged commit fc5ba76 into pac4j:master Mar 25, 2018
@victornoel victornoel deleted the multimodule branch March 25, 2018 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants