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

Edit Using OpenID Connect (OIDC) multitenancy #37584

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

rolfedh
Copy link
Contributor

@rolfedh rolfedh commented Dec 7, 2023

Purpose: Edit Using OpenID Connect (OIDC) multitenancy
References QDOCS-556

@rolfedh
Copy link
Contributor Author

rolfedh commented Dec 7, 2023

I expect to push a few updates to the PR over the next hour, after the CI "[vale] reported by reviewdog" tool generates some additional warnings.

@rolfedh
Copy link
Contributor Author

rolfedh commented Dec 7, 2023

Okay, I'm done pushing changes in response to "reviewdog." This PR is ready for review.

@MichalMaler
Copy link
Contributor

Hello @rolfedh .
I added my review.
Looks good; I added just some minor suggestions, but please, before you request a peer review, make sure you are applying basic flow, such as one sentence per line and so on. Thank you.

Copy link

github-actions bot commented Dec 7, 2023

🙈 The PR is closed and the preview is expired.

@rolfedh
Copy link
Contributor Author

rolfedh commented Dec 7, 2023

Looks good; I added just some minor suggestions, but please, before you request a peer review, make sure you are applying basic flow, such as one sentence per line and so on. Thank you.

Thanks a bunch, @MichalMaler! Your suggestions were spot on and really helpful. Just to loop you in, as we chatted with @sberyozkin and also covered in today's meeting, we're planning to tackle the line breaks as a separate step.

@rolfedh
Copy link
Contributor Author

rolfedh commented Dec 7, 2023

@sberyozkin I have finished applying @MichalMaler's peer review suggestions. This draft is ready for your review.
You're welcome to push back on any of the changes or ask for explanations.
I've tried to optimize the language for clarity and precision while retaining the intended meaning.
In a couple of cases, which I've identified for you, I've proposed alternate content in comments. With your approval, I'll remove the original content and uncomment the alternative content. Thank you!

@rolfedh rolfedh force-pushed the QDOCS-556 branch 2 times, most recently from d024c55 to d8b842b Compare December 7, 2023 19:47
@rolfedh rolfedh removed the request for review from sheilamjones December 7, 2023 19:48
@sberyozkin
Copy link
Member

Hi @rolfedh A lot of good cleanup here, thanks, a few minor suggestions and comments there from me, but overall, very nice, thanks

@rolfedh rolfedh force-pushed the QDOCS-556 branch 2 times, most recently from 233780f to df8456b Compare January 4, 2024 20:06
@rolfedh
Copy link
Contributor Author

rolfedh commented Jan 4, 2024

@sberyozkin If you approve, I believe this is ready for merge.

@MichalMaler
Copy link
Contributor

@rolfedh Added a little tweaks. Feel free to merge when @sberyozkin provides us with the green one.
Cheers!

@rolfedh rolfedh marked this pull request as draft January 8, 2024 13:10
@rolfedh rolfedh marked this pull request as ready for review January 8, 2024 20:37
@rolfedh rolfedh marked this pull request as draft January 8, 2024 20:37
@rolfedh
Copy link
Contributor Author

rolfedh commented Jan 8, 2024

This PR is ready for a final review by @sberyozkin. Thanks.

@rolfedh rolfedh marked this pull request as ready for review January 16, 2024 12:07
@sberyozkin
Copy link
Member

@rolfedh LGTM overall, minor suggestions proposed

Copy link
Contributor Author

@rolfedh rolfedh left a comment

Choose a reason for hiding this comment

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

@sberyozkin Thank you for your review comments. I've made updates accordingly. Please let me know if any further improvements are needed.

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.

Thanks @rolfedh

@sberyozkin sberyozkin merged commit 79c19bd into quarkusio:main Jan 16, 2024
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants