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 for 2.x #38

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
  • Users can still depend on org.pac4j.jax-rs-pac4j for backward compatibility
  • Add saveInSession parameter for callback endpoints

cf #37 for details on why

@pierre-l here it is for 2.x

@leleuj, two points:

  • the difference with Use a multimodule maven organisation #37 is that there is an extra project org.pac4j.jax-rs-pac4j for backward compatibility
  • I understood from the release note said that pac4j 2.3.0 had the new saveInSession parameter for callback endpoints but it isn't there in CallbackLogic.perform, instead I set it on the logic itself, could you confirm that what I did is correct?

@victornoel victornoel requested a review from leleuj March 18, 2018 13:36
@victornoel victornoel added this to the 2.2.0 milestone Mar 18, 2018
@victornoel victornoel self-assigned this Mar 18, 2018
@victornoel victornoel force-pushed the multimodule-2.x branch 3 times, most recently from 8bff754 to df716a4 Compare March 18, 2018 15:06
@leleuj
Copy link
Member

leleuj commented Mar 21, 2018

OK. This one targets an existing branch, so yes, it makes sense to keep the existing jax-rs-pac4j library.

Yes, pac4j v2.3.0 has a saveSession property in the DefaultCallbackLogic and not in the perform method to keep backward compatiblity.

Copy link
Member

@leleuj leleuj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me. One minor question.

*
* @return value for {@link CallbackFilter#setSaveInSession(Boolean)}
*/
boolean[] saveInSession() default {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using Boolean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would you want something that can have the value null here?

The point of those annotations is that they allow the user to express themselves without over thinking: if they put a value it is used, if they do not, the default is used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

 - Bump to pac4j 2.3.0
 - Users should depend on jersey-pac4j or resteasy-pac4j
 - Users can still depend on jax-rs-pac4j for backward compatibility
 - 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 :)

@victornoel victornoel merged commit 5b84990 into pac4j:2.x Mar 25, 2018
@victornoel victornoel deleted the multimodule-2.x 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.

2 participants