Skip to content

Use classpathFromResources in InexpensiveLogStatements #217

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
Merged
merged 3 commits into from
May 6, 2025
Merged

Use classpathFromResources in InexpensiveLogStatements #217

merged 3 commits into from
May 6, 2025

Conversation

JohannisK
Copy link
Contributor

What's changed?

Use classpathFromResources instead of classpath

@JohannisK JohannisK self-assigned this May 6, 2025
@JohannisK JohannisK added the bug Something isn't working label May 6, 2025
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite May 6, 2025
@JohannisK
Copy link
Contributor Author 

recipeDependencies {
    parserClasspath("org.slf4j:slf4j-api:2.+")
    ...
}

I'm assuming this already covers the dependency being on the classpath? Or should I add parserClasspath("org.slf4j:slf4j-api:2.1.+")

github-actions[bot]
github-actions bot reviewed May 6, 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:

  • src/main/resources/META-INF/rewrite/examples.yml
    • lines 29-30
    • lines 38-39
    • lines 54-56
    • lines 64-66
    • lines 735-734
    • lines 781-800

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite May 6, 2025
github-actions[bot]
github-actions bot reviewed May 6, 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:

  • src/main/resources/META-INF/rewrite/examples.yml
    • lines 29-30
    • lines 38-39
    • lines 54-56
    • lines 64-66
    • lines 735-734
    • lines 781-800

@timtebeek
Copy link
Member 

recipeDependencies {
    parserClasspath("org.slf4j:slf4j-api:2.+")
    ...
}

I'm assuming this already covers the dependency being on the classpath? Or should I add parserClasspath("org.slf4j:slf4j-api:2.1.+")

You'll want to match what's used in parserClasspath with what's used in classpathFromResources. Otherwise you might find that the next time that the type table is generated after a new say 2.2 version of slf4j-api comes out your recipe tests will break.

@JohannisK JohannisK merged commit 05ebc23 into main May 6, 2025
@JohannisK JohannisK deleted the feature/inexpensivelogstatements_use_classpath_from_resources branch May 6, 2025 08:27
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite May 6, 2025
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

@jevanlingen jevanlingen jevanlingen approved these changes

Assignees

@JohannisK JohannisK

Labels
bug Something isn't working
Projects
OpenRewrite
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JohannisK @timtebeek @jevanlingen