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

Fixes retrieval of ModuleComponentIdentifier #74

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

jvican
Copy link
Contributor

@jvican jvican commented Oct 27, 2023

For some reason, the Bloop plugin wouldn't work at all in a private repository using a proprietary version of Gradle. I believe this is because the presence of a certain plugin that was interfering and changing the artifacts available after resolution to be something else than ModuleComponentArtifactIdentifier, so the Scala plugin would never collect the Scala library. But, the result was that the Bloop plugin wouldn't generate Bloop files and it would fail altogether.

The following change, which is untested because I couldn't reproduce, has a fix that uses a more general interface ModuleComponentIdentifier and pattern matches on getComponentIdentifier instead of the regular artifact identifiers. This seems to be the most correct and general way of writing the previous logic.

For some reason, the Bloop plugin wouldn't work at all in a private
repository using a proprietary version of Gradle. I believe this is
because the presence of a certain plugin that was interfering and
changing the artifacts available after resolution to be something else
than `ModuleComponentArtifactIdentifier`, so the Scala plugin would
never collect the Scala library. But, the result was that the Bloop
plugin wouldn't generate Bloop files and it would fail altogether.

The following change, which is untested because I couldn't reproduce,
has a fix that uses a more general interface `ModuleComponentIdentifier`
and pattern matches on `getComponentIdentifier` instead of the regular
artifact identifiers. This seems to be the most correct and general way
of writing the previous logic.
@Arthurm1
Copy link
Contributor

If it passes the tests then LGTM. I'm never sure of the correct way of doing anything with the Gradle API 🤷

Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

Looks good but it needs formatting

@jvican
Copy link
Contributor Author

jvican commented Oct 31, 2023

@Arthurm1 Don't worry about it, it's hard!

@adpi2 Updated!

@jvican
Copy link
Contributor Author

jvican commented Oct 31, 2023

@adpi2 Could you also please cut a release after merging?

@adpi2 adpi2 merged commit c5aa3b8 into scalacenter:main Nov 1, 2023
3 checks passed
@adpi2
Copy link
Member

adpi2 commented Nov 1, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants