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

TypeTable should store actual version for predictable matching #77

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

timtebeek
Copy link
Contributor

What's changed?

Determine the actual jar version number, as opposed to the requested version number.

What's your motivation?

We saw mismatches in

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

Called out in comments. Tagged Sam for async review.

Have you considered any alternatives or workarounds?

We could adjust the matching as well, but then we'd be storing ../5.+/.. on disk, which could be 5.0 - 5.3. This avoids confusion.

@timtebeek timtebeek requested a review from sambsnyd February 14, 2025 16:24
@timtebeek timtebeek self-assigned this Feb 14, 2025
Comment on lines +54 to +56
String version = dependency.getValue().getName()
.substring(artifact.length() + 1)
.replaceAll(".jar$", "");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API exposed by Gradle here is quite minimal, only showing the requested version. This naive wrangling I expect to mostly work, except in rare cases that I do not expect to encounter in parserClasspath entries just yet. Open to improvements.

@timtebeek timtebeek added the bug Something isn't working label Feb 14, 2025
@timtebeek timtebeek merged commit 9a48d38 into main Feb 14, 2025
1 check passed
@timtebeek timtebeek deleted the type-table-should-store-actual-version branch February 14, 2025 16:28
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.

1 participant