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

Bump the version using the #flags from the PR description #85

Merged
merged 8 commits into from
Nov 21, 2024

Conversation

ll-nick
Copy link
Collaborator

@ll-nick ll-nick commented Nov 20, 2024

Closes #79

Instead of relying on a marketplace action, we now use bash script to compute a new version from the PR description. The new version is computed based on the existence of one of the following flags (proceeded by a #):

  • major
  • minor
  • patch

#patch

@ll-nick ll-nick requested a review from orzechow November 20, 2024 14:19
@ll-nick ll-nick self-assigned this Nov 20, 2024
@ll-nick
Copy link
Collaborator Author

ll-nick commented Nov 20, 2024

I tested this using another branch where the workflow is triggered on any push, the version is computed from the commit description. The pipeline also stops after echoing the new version in the next job in order not to generate a bunch of useless releases and automated commits.

Check out the action for a test of each case (major, minor, patch, default)

Copy link
Member

@orzechow orzechow left a comment

Choose a reason for hiding this comment

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

Thanks! That was a quick one 👌

Please make sure that this solution is not vulnerable to code injections. Is the PR body escaped or would a PR body starting with e.g. " break the job?

.github/workflows/compute_version.sh Outdated Show resolved Hide resolved
@ll-nick
Copy link
Collaborator Author

ll-nick commented Nov 21, 2024

Ok, this is ready for re-review @orzechow

On top of the points in the discussions above, I tidied up the script and the workflow a little in 95bcfb3, e7934b9 and 228c042

For reference, I triggered the action using the test branch again, this time using a malicious commit message containing a ".
It failed at first, but after adding the sanitation in the workflow (20e87b8) it worked fine.

@ll-nick ll-nick requested a review from orzechow November 21, 2024 07:05
Copy link
Member

@orzechow orzechow left a comment

Choose a reason for hiding this comment

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

Great! 😁

I guess the last nitpick:
We could think how to avoid the #tag from ending up in our release description.
Not super important though 🙈

.github/workflows/compute_version.sh Outdated Show resolved Hide resolved
@ll-nick
Copy link
Collaborator Author

ll-nick commented Nov 21, 2024

Thanks for the review. About your point: I'd say the benefit is not worth the effort. It adds another potential point of failure to a workflow that's pretty complex already and I don't think it hurts having it there. So I'll just go ahead and merge as is :)

@ll-nick ll-nick merged commit 45fc92f into main Nov 21, 2024
1 check passed
@ll-nick ll-nick deleted the version_bump_via_pr_description branch November 21, 2024 16:04
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.

GitHub version bump action always bumps at least minor
2 participants