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

Enhance the functionality of the bump_version.sh script #169

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Aug 10, 2023

🗣 Description

This pull request adds some functionality to the bump_version.sh script and DRYs out some of the logic.

Note

If/when this pull request is approved I plan on porting the changes over to other skeletons that have a bump_version.sh script.

💭 Motivation and context

I have used custom tokens for prerelease versions in some of my projects (dev instead of rc as an example) to better articulate what the version represents. I thought about it and determined that building the functionality into the script would be useful. While I was making the changes to support custom tokens I realized it would also make sense to do some general cleanup of the script.

🧪 Testing

Automated tests pass. I verified that the script works as expected both with and without custom tokens for build/prerelease actions. I also verified that other actions worked as expected.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

The token is optionally used for the build and prerelease actions to
override the default values used by the semver Python package.
Since the replacement and commit of version change information is
almost identical across the different logic flows it makes sense to DRY
out the code.
Only retrieve the existing version information if the input to the
bump_version.sh script isn't immediately invalid (if the script does
not get 1-2 arguments).
@mcdonnnj mcdonnnj added the improvement This issue or pull request will add or improve functionality, maintainability, or ease of use label Aug 10, 2023
@mcdonnnj mcdonnnj self-assigned this Aug 10, 2023
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

This looks like a nice improvement, but I want to get clarification on one thing before I approve.

bump_version.sh Show resolved Hide resolved
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

👍 👍

Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

I made one suggestion, which need not impede the approval of this pull request.

bump_version.sh Outdated Show resolved Hide resolved
The team has come to the consensus that the `git push` should be at the
discretion of the user. This both prevents needless GitHub Actions runs
and ensures that the repository is updated when the user feels it is in
an appropriate state.

Co-authored-by: Shane Frasier <[email protected]>
@mcdonnnj mcdonnnj added the kraken 🐙 This pull request is ready to merge during the next Lineage Kraken release label Aug 16, 2023
@mcdonnnj mcdonnnj added this pull request to the merge queue Dec 6, 2023
Merged via the queue into develop with commit bf95545 Dec 6, 2023
13 checks passed
@mcdonnnj mcdonnnj deleted the improvement/enhance_bump_version_script branch December 6, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use kraken 🐙 This pull request is ready to merge during the next Lineage Kraken release
Projects
Development

Successfully merging this pull request may close these issues.

4 participants