-
Notifications
You must be signed in to change notification settings - Fork 255
feat(cmd-version): add support for partial tags #1115
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
base: master
Are you sure you want to change the base?
feat(cmd-version): add support for partial tags #1115
Conversation
@MatthieuSarter, thanks for all the hard work you have put in here. It looks good and I was not thinking about this kind of feature or how to implement. One suggestion would be to change the configure name to a longer more descriptive name Thanks for adding the descriptions and doc updates and timeline for testing rather than just passing it to me to complete (surprising how many people do this). To help understand the test suite, note there is the tests/ directory that is split into unit & e2e folders and how to run the test suite is documented in Recommend It is probably also worth noting that a I did swap your PR to ready for review to see how your changes currently evaluate in the CI and nothing failed so you effectively added functionality without breaking anything (that we know of) which is awesome. Now we would just be looking for tests that exercise your new functionality (tag creations & updates) and any new ways that it would impact other non-default settings or environments. Good job so far! |
@codejedi365 , thanks for your explanation, it will be helpfull to help me add tests :) Regarding the name of the option, I used this name as it is the name I commonly encountered on other projects for naming such tags, especially in the Docker world (see here for instance: https://docs.vmware.com/en/VMware-Tanzu-Application-Catalog/services/main/GUID-apps-tutorials-understand-rolling-tags-containers-index.html ), but if you prefer an other terminology, I'm totally fine with it. Among your two suggestions, I would prefer rolling_partial_tags, which would both indicate that it enable rolling tags and that those tags are based on partial version (as opposed to rolling tags like "latest", without any version infos). |
I appreciate the insight and thought considered for the name. As I continued writing my response I think I have settled on the |
@MatthieuSarter, Thinking about the tests a little more, I think it will be important to verify the following situations:
Question: how would we consider handling a build metadata change release or just a release with build metadata? I feel like we should be creating a "partial" release from the full tag which would result in a Note: I recommend using the fixture |
the force push change was to help resolve the conflict in |
@codejedi365 , just a little message to tell you I didn't forgot about my commitment to add tests to this PR, but I had a tough start of the year (flu, bike crash...) so I didn't have the opportunity to work on it yet. |
@MatthieuSarter, thanks for the note, I was starting to debate if you would follow through as most just leave it to me to pick up where they left off. I'm glad you haven't. I hope you feel better soon! It sounds like a rough start to 2025 for you--must be frustrating. If you have seen the rebases, I have just been keeping what was written up to date with the latest version as stuff has refactored which should make it easier to pick up from. |
@codejedi365, I finally had some time to start adding tests 🙂 Not much for now, but it's a start. I used test_version_bump.py::test_version_force_level as reference, and kept only what is pertinent for tag related checks (ie. dropped the check on version update in files, release creation in VCS, etc...). Regarding the way of handling partial tag for prerelease, I think the answer is no update of the partial tags in this cases, as the partial tags should always point to a release, not a prerelease. For now I opted for the same behavior in case of version with build metadata, as for me those versions are not release. But I'm not used to this versioning scheme, so I'm open to discussion about this, creating/updating partial tags up to the patch level also seems to be a reasonable option in this case. |
@MatthieuSarter, Glad to hear!
I would rather you not drop the assertions in e2e tests. In a e2e test of version, they are all pertinent. As I stated above, it is on purpose to have all the checks so we know that this variation doesn't have a weird side impact on something we didn't consider.
I concur that should be the outcome. We will want a test to ensure these do not move.
Build metadata releases are official releases, they just include more specific information (generally the date). Note that the
|
Set rolling_tags to true to create/update major and major.minor tags when releasing a version. Also remove warnings about invalid version for the rolling tags.
…th build metadata
Purpose
Add a new option
add_partial_tags
to handle creation and update of partial tags for major and major.minor.Rationale
It will simplify the CI process for project using partial tags, as it will no more require to add scripts to update those after performing a version bump with psr.
How did you test?
For now, I only did manual testing. I need more time to setup a fully working test environment and investigate the way the tests are written. That's why the PR is opened as draft for now, just so you know that I'm working on the feature and can have a first look at it. I should have some spare time at the end of the month or in january to look at the tests.
How to Verify
add_partial_tags
totrue
in configuration.Sample output without partial tags enabled:
And the same with rolling tags enabled: