Skip to content

Conversation

stuartwdouglas
Copy link
Collaborator

No description provided.

@stuartwdouglas stuartwdouglas added the kind/bug Something isn't working label Oct 30, 2019
@gsmet gsmet added this to the 0.28.0 milestone Oct 30, 2019
return new FeatureBuildItem(FeatureBuildItem.OIDC);
}

@BuildStep
Copy link
Member

Choose a reason for hiding this comment

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

Is using capabilities to do this preferable to a common extension that they both depend on?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding a new module for 3 lines of code seems like overkill.

Copy link
Member

Choose a reason for hiding this comment

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

It's mostly about whether it's intuitive for those coming along later that if there are additional beans that need to be added for JWT in the SmallRye JWT extension, that it also needs to be added to this code block

Copy link
Member

Choose a reason for hiding this comment

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

@kenfinnigan Hi Ken it is a good point. We have an issue to link the OIDC with the quarkus-smallrye-jwt (in which case this block would be redundant), but I'm not sure yet if a link between these 2 modules is needed or not, and may be just having smalrye-jwt being the shared library is the right way to go with some small duplication as in this build step.

@sberyozkin sberyozkin self-requested a review October 31, 2019 09:04
Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

@stuartwdouglas FYI, there is CDI code directly in quarkus-smallrye-jwt required to have some more MP JWT injection working - but I'm going to work on abstracting as much as possible from the code to smallrye-jwt

@sberyozkin sberyozkin merged commit b40baaa into quarkusio:master Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants