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

MODULE: oauth2 #2755

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

MODULE: oauth2 #2755

wants to merge 7 commits into from

Conversation

hosuaby
Copy link
Contributor

@hosuaby hosuaby commented Feb 5, 2025

Integration of OAuth2/OpenID flows into feign.
Following the discution: #2680

@hosuaby
Copy link
Contributor Author

hosuaby commented Feb 5, 2025

@velo

Ah, build fails cause I am using Testcontainers. What do you suggest? Do I disable all Tests using Testcontainers or there is a way to get a build environment with Docker on CircleCI?

@velo
Copy link
Member

velo commented Feb 9, 2025

Well, I would say to either test using something else. Or figure how to get circle Ci working with test containers or need to move to GitHub actions

@hosuaby
Copy link
Contributor Author

hosuaby commented Feb 9, 2025

Well, I would say to either test using something else. Or figure how to get circle Ci working with test containers or need to move to GitHub actions

I am doing experiments with pipeline. Please ignore notifications. When I will come up with a solution, I will inform you and make a separate PR.

@hosuaby hosuaby mentioned this pull request Feb 9, 2025
Merged
velo
velo approved these changes Feb 10, 2025
View reviewed changes
Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

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

Amazing work, tons of tests, nice improvements, no breaking change, sounds like a win win

src/docs/overview-mindmap.iuml
@@ -41,6 +41,7 @@ left side
*** Dropwizard Metrics 5
*** Micrometer
** extras
*** OAuth2
Copy link
Member

Choose a reason for hiding this comment

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

NICE!

pom.xml Outdated
<artifactId>feign-vertx</artifactId>
<version>${project.version}</version>
</dependency>

<dependency>
Copy link
Member

Choose a reason for hiding this comment

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

thanks for the clean up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, removed this addition. Anyway it will be added in my PR about Vertx.

@velo velo requested a review from kdavisk6 February 10, 2025 14:37
github-actions[bot]
github-actions bot reviewed Feb 10, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • annotation-error-decoder/pom.xml
    • lines 49-48
  • apt-test-generator/pom.xml
    • lines 62-61
  • benchmark/pom.xml
    • lines 70-69
  • core/pom.xml
    • lines 37-36
  • googlehttpclient/pom.xml
    • lines 54-53
  • hc5/pom.xml
    • lines 67-66
  • httpclient/pom.xml
    • lines 75-74
  • hystrix/pom.xml
    • lines 78-77
  • jackson-jaxb/pom.xml
    • lines 82-83
  • java11/pom.xml
    • lines 50-49
  • jaxb-jakarta/pom.xml
    • lines 55-57
  • jaxb/pom.xml
    • lines 45-49
  • jaxrs2/pom.xml
    • lines 103-102
  • jaxrs3/pom.xml
    • lines 89-88
  • jaxrs4/pom.xml
    • lines 83-82
  • kotlin/pom.xml
    • lines 71-70
  • okhttp/pom.xml
    • lines 59-58
  • pom.xml
    • lines 614-615
  • reactive/pom.xml
    • lines 98-97
  • ribbon/pom.xml
    • lines 83-82
  • soap-jakarta/pom.xml
    • lines 63-62
    • lines 79-81
  • soap/pom.xml
    • lines 55-60
    • lines 70-71

@kdavisk6
Copy link
Member

Circle should be disabled now. Please update the branch and we'll try again.

@hosuaby
Copy link
Contributor Author

hosuaby commented Feb 25, 2025

Hello
@velo @kdavisk6

Don't wanna be pushy, but I would like to know when review will be done? I have already multiple PRs and more will come very soon.

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

@github-actions github-actions[bot] github-actions[bot] left review comments

@velo velo velo approved these changes

@kdavisk6 kdavisk6 Awaiting requested review from kdavisk6

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

Successfully merging this pull request may close these issues.
3 participants
@hosuaby @velo @kdavisk6