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.

Sign up for GitHub

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

Jump to bottom

Take conditional annotation into account for @ServerExceptionMapper #29459

Merged
merged 1 commit into from
Dec 5, 2022
Merged

Take conditional annotation into account for @ServerExceptionMapper #29459

merged 1 commit into from
Dec 5, 2022

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Nov 24, 2022

Closes: #29043

@geoand geoand mentioned this pull request Nov 24, 2022
Closed
@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand geoand requested a review from Sgitario November 24, 2022 10:49
@ivansenic ivansenic mentioned this pull request Nov 24, 2022
Closed
Sgitario
Sgitario reviewed Dec 5, 2022
View reviewed changes
Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

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

The changes look good to me, though the CI is broken due to #29403.

Moreover, I would consider adding a section to note/describe that we can use these annotations in the exception mappers.

@geoand
Copy link
Contributor Author

geoand commented Dec 5, 2022

Moreover, I would consider adding a section to note/describe that we can use these annotations in the exception mappers.

I have actually found that users intuitively think that all annotations work together, unless otherwise stated, so I don't think we need to explicitly mention anything.

I rebased the PR onto main to get another CI run

@geoand geoand added triage/waiting-for-ci Ready to merge when CI successfully finishes triage/backport? labels Dec 5, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 5, 2022

Failing Jobs - Building 17eecf2

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
✖ JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@geoand
Copy link
Contributor Author

geoand commented Dec 5, 2022

@Sgitario can I get a review please?

Thanks

@geoand geoand removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 5, 2022
Sgitario
Sgitario approved these changes Dec 5, 2022
View reviewed changes
Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

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

lgtm!

@geoand geoand merged commit fddf257 into quarkusio:main Dec 5, 2022
@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Dec 5, 2022
@geoand geoand deleted the #29403 branch December 5, 2022 11:18
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 5, 2022

Milestone is already set for some of the items:

We haven't automatically updated the milestones for these items.

This message is automatically generated by a bot.

@gsmet gsmet modified the milestones: 2.16 - main, 2.15.0.Final Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sgitario Sgitario Sgitario approved these changes

Assignees
No one assigned
Labels
area/rest kind/bugfix triage/flaky-test
Projects
None yet
Milestone
2.15.0.Final
Development

Successfully merging this pull request may close these issues.

Parent first dependencies method in Class Loading Reference guide not working for applications
3 participants
@geoand @Sgitario @gsmet