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 how next version is calculated during pre-release #320

Merged

Conversation

breedx-splk
Copy link
Contributor

We append -SNAPSHOT in all non -Pfinal=true builds, so this is not needed.

@breedx-splk breedx-splk requested a review from a team April 23, 2024 22:04
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

another option is to adopt the version management strategy (and automation) from the other java repos, which also append -SNAPSHOT to the version, but do it more explicitly, e.g.

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/version.gradle.kts

@breedx-splk
Copy link
Contributor Author

breedx-splk commented Apr 24, 2024

another option is to adopt the version management strategy (and automation) from the other java repos, which also append -SNAPSHOT to the version, but do it more explicitly, e.g.

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/version.gradle.kts

Thanks, yeah I appreciate that. I can't remember the details about it right now, but I thought there was something Android-specific (AGP?) that preferred having the version in settings, and maybe having suffixes there caused problems. I can't remember.

@LikeTheSalad do you remember?

I any case, I think that's a much bigger change at this point, and I'm giving preference for keeping it small right now.

@breedx-splk breedx-splk merged commit 1dbb49c into open-telemetry:main Apr 24, 2024
3 checks passed
@LikeTheSalad
Copy link
Contributor

Thanks, yeah I appreciate that. I can't remember the details about it right now, but I thought there was something Android-specific (AGP?) that preferred having the version in settings, and maybe having suffixes there caused problems. I can't remember.

@LikeTheSalad do you remember?

I any case, I think that's a much bigger change at this point, and I'm giving preference for keeping it small right now.

I think the current logic to append -SNAPSHOT predates OTel Android so I'm not sure about the reason behind it, however, I don't think it's a bad approach, and I agree that there are already a couple of parts built around that logic so... The benefit that I see of using the same approach as the other repos is WYSIWYG for the build version, however, I think it shouldn't be too strange to get -SNAPSHOT appended out of the blue in a non-final build so I'm not sure if it's worth making the changes for that reason. Though now that we're on the topic, I do believe it's quite unexpected to get -alpha appended out of the blue, which is something we're currently doing as well 😅 so maybe we should consider removing that logic and just set the version inside gradle.properties to x.x.x-alpha.

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