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 bump workflow #49

Merged
merged 9 commits into from
Oct 2, 2023
Merged

Add bump workflow #49

merged 9 commits into from
Oct 2, 2023

Conversation

Tyrrrz
Copy link
Contributor

@Tyrrrz Tyrrrz commented Oct 2, 2023

This adds a new, dispatchable workflow called bump. You can use it to update the current package version (of both Passwordless and Passwordless.AspNetCore). It does that by updating Directory.Build.props, committing changes to a new branch, and then creating a pull request. Very similar to what @abergs did in #27.

In order to facilitate this workflow, I also took the liberty of performing one additional breaking change: auto-generated prerelease package versions for CI deployments now follow the pattern of 0.0.0-ci-BRANCH-RUNID. The main difference is that the version is always 0.0.0.

This change was done so that we could consolidate both VersionPrefix and VersionSuffix into a single Version property. It's not essential for this workflow to work, but then instead of one input it would likely take two (which is not a catastrophe).

However, here are some advantages to this versioning schema:

  • Previously, auto-generated prerelease version was attached to the current version prefix. That means, if Passwordless version 2.0.0 was already released, all subsequent CI builds would push a package with a version like 2.0.0-ci-main-12345. This meant that, in terms of semantic versioning, they appeared older than the stable 2.0.0 package, which was actually released earlier.
    • This could be resolved by bumping the package version preemptively, right after a release. In other words, right after 2.0.0 is released, we would update the current version to 2.0.1 or something. The issue here is that we can't know ahead of time which component we should be bumping. The prerelease versions will be ordered correctly this way, but it feels weird to have a semantic version that's not actually semantic.
    • Not bumping preemptively has the added benefit of preventing us from accidentally releasing a version that's not ready for release. For example if we just released 2.0.0 and some workflow or user error resulted in another deployment of that package, it would fail because versions are immutable. However, if we preemptively bumped the version to 2.0.1, the erroneous deployment would go through.
    • For reasons above, I believe 0.0.0-ci-BRANCH-RUNID is a better naming schema. However, we might want to prepend the suffix with the current unix timestamp to make sure the prerelease versions are also ordered correctly between themselves.

Again, if the version change is undesired, I can revert that part and change the bump workflow to take two inputs instead of one.

Comment on lines 15 to 37
# Ensure that the provided version is greater than the current version
validate:
runs-on: ubuntu-latest
permissions:
contents: read

steps:
- name: Checkout
uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Get current version
id: current-version
run: |
props_file=$(cat Directory.Build.props)
version=$(echo "$props_file" | grep -oPm1 "(?<=<Version>)[^<]+")
echo "version=$version" >> $GITHUB_OUTPUT

- name: Validate new version
run: |
if npx semver "${{ inputs.version }}" --range "<= ${{ steps.current-version.outputs.version }}"; then
echo "New version must be greater than the current version"
exit 1
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This uses the semver tool to make sure that the new version is valid and greater than the current version

@abergs
Copy link
Member

abergs commented Oct 2, 2023

Hmm... I have mixed feelings about not bumping preemptively. I do like added safety harness that autobuilt stuff coming of a major 0.... and the theoritcal scenario of using a CI built package and loosking track of which SDK version you're using might simply be a 0.001%.

I also like the tidyness of bumping preemptively, but either way I'm OK with the changes in this PR.

If we later change or mind, we can just switch it up.

@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Oct 2, 2023

Hmm... I have mixed feelings about not bumping preemptively. I do like added safety harness that autobuilt stuff coming of a major 0.... and the theoritcal scenario of using a CI built package and loosking track of which SDK version you're using might simply be a 0.001%.

I also like the tidyness of bumping preemptively, but either way I'm OK with the changes in this PR.

If we later change or mind, we can just switch it up.

Ultimately, your call. Both approaches have some trade offs. But I'm not attached to either.

So keep the 0.0.0 format?

@abergs
Copy link
Member

abergs commented Oct 2, 2023

For now it's OK, we might reconsider if we experience pain points.

@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Oct 2, 2023

@abergs okay it's ready then. Do I have your approval to merge?

I will want to test it out after this to see if it works. I tested it locally, but unfortunately I can't test dispatch workflows off a branch on GitHub. We can just close the test PR, if that's okay.

@Tyrrrz Tyrrrz merged commit 115ce98 into main Oct 2, 2023
5 checks passed
@Tyrrrz Tyrrrz deleted the automate-version-bump branch October 2, 2023 14:08
@Tyrrrz Tyrrrz mentioned this pull request Oct 2, 2023
@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Oct 2, 2023

It works: #51

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.

2 participants