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

chore: associate a commit with beta release #5530

Merged

Conversation

algomaster99
Copy link
Contributor

I am proposing changes in the beta release script to associate commit with a release so that the reproducibility of all releases on maven central can be checked by reproducible central. They are not able to do so right now because they cannot find the source code corresponding to the release.

I simply just copied the content from release.sh to release-beta.sh to push the commits. However, I am not sure how jreleaser will know which commit to tag as it is on a different branch, but I assume it would work as it works for normal releases.

@MartinWitt
Copy link
Collaborator

Hey,

From my perspective, the beta releases are only a relict of the past, and I am against promoting them as real releases. Beta releases are a random snapshot of the master, and we have no ability to express what this release contains. Our SemVer releases are way better for end user and should be the preferred usage. Since our CD upgrade we do them more often (we should still push more fix releases, but that's another topic). I would prefer to stop releasing them before we add many commits to the master for beta releases.

@algomaster99
Copy link
Contributor Author

I understand your point. I think the beta releases exist because our main release cycles were slow to incorporate a features/fixes on the master branch because Martin M did them manually. Since automating them, I agree that we could completely remove beta releases and just perform normal (SemVer) releases.

@algomaster99
Copy link
Contributor Author

So should I close this PR and submit a PR for removing/disabling beta releases? The goal is that all packages that are pushed to maven central should be reproducible.

@MartinWitt
Copy link
Collaborator

I discussed this topic with @monperrus. We decided to fully commit to beta releases. Every beta release should get a tag and a commit on master. Sorry for the confusion.

@algomaster99
Copy link
Contributor Author

@I-Al-Istannen if you are also in favour of " Every beta release should get a tag and a commit on master", should I submit a PR to merge the two release processes? I would change https://github.com/INRIA/spoon/blob/master/.github/workflows/release-beta.yml#L11 to use release.sh.

@I-Al-Istannen
Copy link
Collaborator

I am not quite how you would merge them, as they use a different naming scheme (but both deploy to maven central, and not sonatype snapshots, right?).

Committing the beta version to master would entail making a commit changing the version to include -beta, which likely breaks the version bumping in the other release scripts, right?

@algomaster99
Copy link
Contributor Author

deploy to maven central, and not sonatype snapshots, right?

Yes. This is why I thought I could merge them. However naming scheme, commit message, and tags would differ like you said. Thanks for pointing it out.

Then I believe reviewing this PR itself would be better.

@algomaster99
Copy link
Contributor Author

Hi! Can we merge this? We want to do it for the sake of reproducibility checks on our beta releases.

Comment on lines 48 to 52
echo "::group::Updating poms to next target version"
mvn -f spoon-pom --no-transfer-progress --batch-mode versions:set -DnewVersion="$NEXT_RELEASE_VERSION" -DprocessAllModules
mvn --no-transfer-progress --batch-mode versions:set -DnewVersion="$NEXT_RELEASE_VERSION" -DprocessAllModules
mvn -f spoon-javadoc --no-transfer-progress --batch-mode versions:set -DnewVersion="$NEXT_RELEASE_VERSION" -DprocessAllModules
echo "::endgroup::"
Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I can see you do not actually update anything as NEXT_RELEASE_VERSION is already identical to whatever was there originally?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah wait, it was changed in between, the messages and names are just confusing. Maybe just drop NEXT_RELEASE_VERSION and rename CURRENT_VERSION to ORIGINAL_VERSION and then use that variable here. And update the messages to say that you are reverting to the old version and not the next target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NEXT_VERSION is the beta version for which the commit needs to be pushed. So I push the commit, after rewritting POMs, in R30-R42.

Next is NEXT_RELEASE_VERSION which is the SNAPSHOT. It will be the same version before the beta version as we don't have SNAPSHOT versions for beta as well. The name of this variable is confusing, a better name would be SNAPSHOT_VERSION_TO_RESET_TO.

P.S. Sorry I did not notice your reviews earlier.

chore/release-beta.sh Show resolved Hide resolved
@I-Al-Istannen
Copy link
Collaborator

@algomaster99 Could you change the names? :)

@algomaster99
Copy link
Contributor Author

@I-Al-Istannen I am sorry. Completely lost context, but I changed what you asked.

Comment on lines 48 to 52
echo "::group::Updating poms to next target version"
mvn -f spoon-pom --no-transfer-progress --batch-mode versions:set -DnewVersion="$NEXT_RELEASE_VERSION" -DprocessAllModules
mvn --no-transfer-progress --batch-mode versions:set -DnewVersion="$NEXT_RELEASE_VERSION" -DprocessAllModules
mvn -f spoon-javadoc --no-transfer-progress --batch-mode versions:set -DnewVersion="$NEXT_RELEASE_VERSION" -DprocessAllModules
echo "::endgroup::"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah wait, it was changed in between, the messages and names are just confusing. Maybe just drop NEXT_RELEASE_VERSION and rename CURRENT_VERSION to ORIGINAL_VERSION and then use that variable here. And update the messages to say that you are reverting to the old version and not the next target.

@algomaster99
Copy link
Contributor Author

@I-Al-Istannen done!

@algomaster99
Copy link
Contributor Author

@I-Al-Istannen let's merge?

@I-Al-Istannen
Copy link
Collaborator

Actually, sorry: Could you maybe just revert the commit (with a custom message ideally) and then push that? Instead of invoking all kinds of maven stuff that might fail somehow?

@I-Al-Istannen
Copy link
Collaborator

If that's annoying, I am also fine enough with this solution

@algomaster99
Copy link
Contributor Author

Actually, sorry: Could you maybe just revert the commit (with a custom message ideally) and then push that? Instead of invoking all kinds of maven stuff that might fail somehow?

I don't understand what you mean. Revert which commit?

@I-Al-Istannen
Copy link
Collaborator

The commit made by the workflow, bumping the version for the release

@algomaster99
Copy link
Contributor Author

Good idea. Can do that.

@algomaster99
Copy link
Contributor Author

@I-Al-Istannen done! I removed maven stuff and I also added a command at the end to delete the remote branch from repository.

@I-Al-Istannen
Copy link
Collaborator

Then let's see what breaks... :^)

@I-Al-Istannen I-Al-Istannen merged commit fa74466 into INRIA:master Aug 1, 2024
12 checks passed
@algomaster99 algomaster99 deleted the make-beta-release-with-commit branch August 2, 2024 07:59
@algomaster99
Copy link
Contributor Author

Hoping my Sunday is not ruined 🤞

@algomaster99
Copy link
Contributor Author

Nothing broke it seems :D

@algomaster99
Copy link
Contributor Author

@I-Al-Istannen should we mark the release as pre-release instead of latest?

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