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

Add MigrateRequestConfig #45

Merged
merged 14 commits into from
Nov 28, 2024
Merged

Add MigrateRequestConfig #45

merged 14 commits into from
Nov 28, 2024

Conversation

SiBorea
Copy link
Contributor

@SiBorea SiBorea commented Nov 28, 2024

What's changed?

I created a MigrateRequestConfig recipe for RequestConfig, it maybe complicated to finish.
Currently only RequestConfig.Builder.setStaleConnectionCheckEnabled(false) is migrated.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@SiBorea
Copy link
Contributor Author

SiBorea commented Nov 28, 2024

@timtebeek Do you have any better idea for line 103 and 144. I suppose this isn't a good way to insert a new variable.

github-actions[bot]

This comment was marked as outdated.

github-actions[bot]

This comment was marked as outdated.

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:

  • src/main/java/org/openrewrite/apache/httpclient5/UsernamePasswordCredentials.java
    • lines 28-29

Co-authored-by: Tim te Beek <[email protected]>
@timtebeek
Copy link
Contributor

That was a neat puzzle there @SiBorea ! To fix most of the type validation issues I've flipped around the order as compared to what you had before; now we first create a new PoolingHttpClientConnectionManager in visitMethodDeclaration (if needed), and then pass that down into visitMethodInvocation in case we need to call setConnectionManager there. I couldn't fully resolve the type issues just yet due to the mix of HC4 and HC5 classes, but I think this is good enough for now. Thanks again for the extensive tests; they make refactoring easy. 🙏🏻

@timtebeek timtebeek merged commit 847a631 into openrewrite:main Nov 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants