Skip to content

introduce get-release-version in docker helper #13006

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

Merged
merged 3 commits into from
Aug 18, 2025

Conversation

eruditmorina
Copy link
Contributor

@eruditmorina eruditmorina commented Aug 13, 2025

Motivation

  • I want to be able to get the release version if HEAD is a release tag via docker-helper.sh.

Changes

  • Introduce get-release-version command which checks if commit is a release, and if so, outputs the version. Otherwise just a message telling that the commit is not a release.

@eruditmorina eruditmorina added the semver: patch Non-breaking changes which can be included in patch releases label Aug 13, 2025
Copy link

github-actions bot commented Aug 13, 2025

Helper Script Tests

32 tests   32 ✅  0s ⏱️
 2 suites   0 💤
 1 files     0 ❌

Results for commit abb4572.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 13, 2025

Test Results - Preflight, Unit

22 107 tests  ±0   20 372 ✅ ±0   6m 25s ⏱️ -9s
     1 suites ±0    1 735 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit abb4572. ± Comparison against base commit 5a920cb.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 13, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 17s ⏱️ +3s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit abb4572. ± Comparison against base commit 5a920cb.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 13, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files  ±0      5 suites  ±0   2h 21m 25s ⏱️ -27s
4 987 tests ±0  4 395 ✅ ±0  592 💤 ±0  0 ❌ ±0 
4 993 runs  ±0  4 395 ✅ ±0  598 💤 ±0  0 ❌ ±0 

Results for commit 1902b14. ± Comparison against base commit 5a920cb.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 13, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 42m 27s ⏱️ -27s
4 628 tests ±0  4 188 ✅ ±0  440 💤 ±0  0 ❌ ±0 
4 630 runs  ±0  4 188 ✅ ±0  442 💤 ±0  0 ❌ ±0 

Results for commit 1902b14. ± Comparison against base commit 5a920cb.

♻️ This comment has been updated with latest results.

Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks, @eruditmorina for the contribution on the docker-helper!
I would love to learn more about your use case. Why do you need to source the helper? Right now the API is very clear and limited. Sourcing the script could mean that you might want to use functions that are not explicitly exposed via the API, which I would be very cautious about (because it might break at any point in time).

@eruditmorina
Copy link
Contributor Author

eruditmorina commented Aug 14, 2025

Why do you need to source the helper?

@alexrashed Currently we can make use of _get_current_version to determine the projects version and _is_release_commit to be used to check whether we need to generate release changelog notes. Since both those functions are not exposed, I am proposing the ability to source the script. Another alternative would be to expose these as commands explicitly while being cautious about other functions. Or not relying on docker-helper.sh for this use-case at all, but this would speed-up a more straightforward solution. Happy to discuss more!

Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks! Honestly, that is actually pretty dangerous, there is a good chance that _is_release_commit might just be changed without any notice, I was actually shortly afraid we just broke your usage with #12943.

Please make an explicit operation and tests for it, which will make sure that we won't break your code without any notice or even knowledge about the usage.
It seems like you want to determine if your HEAD in currently a release, which sounds like a very useful extension to the script! Something like is-release?

@eruditmorina eruditmorina marked this pull request as draft August 14, 2025 15:07
@eruditmorina eruditmorina changed the title minor: patch docker helper script for sourcing introduce get-release-version in docker helper Aug 15, 2025
@eruditmorina eruditmorina marked this pull request as ready for review August 15, 2025 07:36
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks a lot for jumping on the comments! 💯
The change is looking great, I think this makes it absolutely clear that this is a public API of the helper. I just added a comment concerning the exit code (which I think is quite important?). Happy to discuss / help / assist!

Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for following up on the last comment! This is looking great now and should make it super-easy for your use-case to use the helper, while making sure that we don't break it in the long run! 💯

@alexrashed alexrashed merged commit f5935d4 into main Aug 18, 2025
32 checks passed
@alexrashed alexrashed deleted the patch-docker-helper-for-sourcing branch August 18, 2025 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants