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

fix: create dependencyResourceLoaders in 2 passes #4870

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

natedanner
Copy link
Contributor

@natedanner natedanner commented Jan 9, 2025

What's changed?

In scanJar method in rewrite-core/src/main/java/org/openrewrite/config/Environment.java, we are now creating the list of dependencyResourceLoaders in 2 passes to ensure all recipes are resolved.

What's your motivation?

Recipe descriptors were missing due to transitive recipes failing to be resolved.

Anything in particular you'd like reviewers to focus on?

Are there any undesirable side effects from this change?

Anyone you would like to review specifically?

@timtebeek @sambsnyd @kmccarp

Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Thanks for diving in! Was it just by luck of order of dependencies that we didn't encounter this before? Great to have it resolved now.

@timtebeek timtebeek merged commit 1bb9da0 into main Jan 9, 2025
2 checks passed
@timtebeek timtebeek deleted the fix/two-pass-depdendency-resource-loader branch January 9, 2025 11:17
@timtebeek timtebeek added the bug Something isn't working label Jan 9, 2025
@kmccarp
Copy link
Contributor

kmccarp commented Jan 9, 2025

Thanks for diving in! Was it just by luck of order of dependencies that we didn't encounter this before? Great to have it resolved now.

This is the first time we've had multiple levels of dependencies. Before, we had just first-order dependencies. Now, we have multiple depths of dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants