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 logging & improve version detection for plugin{} block usage #371

Merged
merged 3 commits into from
Dec 13, 2024

Conversation

jdaugherty
Copy link
Contributor

No description provided.

@jdaugherty
Copy link
Contributor Author

As part of the profile publishing we discovered that it's possible to leave a version unspecified before the plugin is applied. this change fixes that & adds logging for debugging

@matrei matrei self-requested a review December 13, 2024 08:08
Copy link
Contributor

@matrei matrei left a comment

Choose a reason for hiding this comment

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

I think it is the version property that is actually used by maven-publish so if only projectVersion is set, we need to also set the version property.

@jdaugherty jdaugherty requested a review from matrei December 13, 2024 14:10
@matrei
Copy link
Contributor

matrei commented Dec 13, 2024

I think it is the version property that is actually used by maven-publish so if only projectVersion is set, we need to also set the version property.

@jdaugherty I think this is still an issue. What happens if only projectVersion is set? detectedVersion become projectVersion. But version will still be unspecified. Isn't version the property that is used by maven-publish?

@jdaugherty
Copy link
Contributor Author

jdaugherty commented Dec 13, 2024

I think it is the version property that is actually used by maven-publish so if only projectVersion is set, we need to also set the version property.

@jdaugherty I think this is still an issue. What happens if only projectVersion is set? detectedVersion become projectVersion. But version will still be unspecified. Isn't version the property that is used by maven-publish?

The purpose of this plugin is it needs to determine if it's a release or a snapshot so it can handle the nexus publishing vs snapshot logic. It currently does that based on the project.version property. The issue is if you apply this plugin in the rootProject under the plugins {} block, it will be evaluated before project.version can be set. Worse, we can't do this check in the afterEvaluate because at that point it's too late to apply the nexus plugin.

I added the projectVersion check as a work around since we typically use that variable.

I think the real solution is to move the release state to a variable and then we wouldn't need the version. Basically, we could do those checks outside of gradle. I was hesitant to make this change for this release since it means every action has to change again. Although, I think realistically we always know based on the action if it's a release or snapshot so I really like this idea instead of relying on the version #.

@jdaugherty jdaugherty merged commit 97c801c into grails:7.0.x Dec 13, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants