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

[WFLY-17649] Add a detailed quickstart for bearer authentication use cases #546

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[WFLY-17649] Add a detailed quickstart for bearer authentication use cases #546

wants to merge 1 commit into from

Conversation

gaol
Copy link
Contributor

@gaol gaol commented Nov 30, 2023

@gaol
Copy link
Contributor Author

gaol commented Nov 30, 2023 •
edited
Loading

@fjuma @emmartins may I ask you to review ? thanks :)

@gaol gaol mentioned this pull request Nov 30, 2023
Open
fjuma
fjuma reviewed Nov 30, 2023
View reviewed changes
quickstarts/WFLY-17649_EJB_Bearer_Token_Authentication_Quickstarts.adoc

* NONE

== Requirements
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to mention whether or not the quickstart will be able to run on OpenShift.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the running on OpenShift to a nice to have requirement, I will try to figure out the Helm Charts for this quickstart.

fjuma
fjuma reviewed Nov 30, 2023
View reviewed changes
quickstarts/WFLY-17649_EJB_Bearer_Token_Authentication_Quickstarts.adoc Outdated

== Test Plan

*
Copy link
Contributor

@fjuma fjuma Nov 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The quickstart should include basic runtime testing and possibly specific runtime testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I updated the test plan section to adding some testing, and also the github action set up for testing against this quickstart.

@gaol
Copy link
Contributor Author

gaol commented Dec 1, 2023

@fjuma thank you for the review, I updated accordingly. :)

fjuma
fjuma approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@fjuma fjuma left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjuma fjuma fjuma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gaol @fjuma