Skip to content

Conversation

michalvavrik
Copy link
Member

Requested here #47457, stop using deprecated fields marked for removal, use accessors instead.

This comment has been minimized.

Copy link

github-actions bot commented Apr 28, 2025

🎊 PR Preview a6fefd7 has been successfully built and deployed to https://quarkus-pr-main-47575-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

This comment has been minimized.

@michalvavrik michalvavrik requested a review from sberyozkin April 28, 2025 18:06
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.

Hi Michal @michalvavrik

Looks perfect, thanks for simplifying and starting with this update only.

I have one minor suggestion, apologies for being annoying :-), can you please drop a OidcTenantConfig#isUserInfoRequired ? The shortcut in itself is neat an makes perfect sense, but it is not an actual ConnfigMapping property declaration.

You have put a lot of effort to clean things up with the legacy OidcTenantConfg, I'd just like to keep that ConfigMapping interface without any extra helpers, etc.
I agree it is a bit annoying to have .orElse(false) - but with this default method we don't know if it was set to anything by the user - which at some point might become relevant

@sberyozkin
Copy link
Member

I'm not sure we want to keep Optional<Boolean> userInfoRequired(); and boolean isUserInfoRequired in one interface...

@michalvavrik
Copy link
Member Author

I don't share your opinion on isUserInfoRequired method, I think it makes sense, but I'll drop it in few moment.s

@michalvavrik michalvavrik force-pushed the feature/oidc-only-access-fields-using-accessors branch from bc71832 to 45885b3 Compare April 29, 2025 13:20

This comment has been minimized.

This comment has been minimized.

@michalvavrik michalvavrik force-pushed the feature/oidc-only-access-fields-using-accessors branch from 45885b3 to 629f3c6 Compare April 29, 2025 14:36
Copy link

quarkus-bot bot commented Apr 29, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 629f3c6.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Apr 29, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 629f3c6.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@sberyozkin sberyozkin merged commit 6315150 into quarkusio:main Apr 29, 2025
30 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.23 - main milestone Apr 29, 2025
@michalvavrik michalvavrik deleted the feature/oidc-only-access-fields-using-accessors branch April 29, 2025 15:40
@gsmet gsmet modified the milestones: 3.23 - main, 3.22.1 Apr 29, 2025
@jmartisk jmartisk modified the milestones: 3.22.1, 3.20.2 Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants