Skip to content

Change storage and common version files to emulate image's#867

Open
TomSweeneyRedHat wants to merge 1 commit into
containers:mainfrom
TomSweeneyRedHat:dev/tsweeney/moveversions
Open

Change storage and common version files to emulate image's#867
TomSweeneyRedHat wants to merge 1 commit into
containers:mainfrom
TomSweeneyRedHat:dev/tsweeney/moveversions

Conversation

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

Change the version file for c/common and c/storage to emulate the version file that is in c/image. This updates version/version.go in c/common, and moves the VERSION file in c/storage to version/version.go.

@github-actions github-actions Bot added storage Related to "storage" package common Related to "common" package labels May 21, 2026
@TomSweeneyRedHat
Copy link
Copy Markdown
Member Author

I tried searching for version to see if these changes might mess up test, but ended up with a bazillion hits from grep. Let's see what the tests turn up.

@packit-as-a-service
Copy link
Copy Markdown

Packit jobs failed. @containers/packit-build please check.

Comment thread common/version/version.go
Comment on lines +6 to +15
// VersionMajor is for an API incompatible changes
VersionMajor = 0
// VersionMinor is for functionality in a backwards-compatible manner
VersionMinor = 69
// VersionPatch is for backwards-compatible bug fixes
VersionPatch = 0

// VersionDev indicates development branch. Releases will be empty string.
VersionDev = "-dev"
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not care that much but personally I find a single string much much simpler than this unnecessary splitting

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As a general matter I dislike strings and especially string parsing, so providing numeric constants (which don’t take any runtime space) is nice.

[Running Sprintf at runtime, on every startup, is a bit less nice, but definitely not worth over-engineering to avoid.]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a general matter I dislike strings and especially string parsing, so providing numeric constants (which don’t take any runtime space) is nice.

[Running Sprintf at runtime, on every startup, is a bit less nice, but definitely not worth over-engineering to avoid.]

Well but this is the problem, you now end up with a non constant string.
A public string can be overwritten by anyone so now the API can be abused, unlikely that we would care but still.

Copy link
Copy Markdown
Contributor

@mtrmac mtrmac May 26, 2026

Choose a reason for hiding this comment

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

I’m a sucker for technically-interesting over-engineering, so:
0001-PROTOTYPE-Generate-a-version-string.patch

The generator would need to be moved elsewhere and shared across packages.

Is this really worthwhile, though? I’m leaning towards thinking this patch is best discarded.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The generator would need to be moved elsewhere and shared across packages.

… and integrated into Makefile to keep the values up-to-date.

@TomSweeneyRedHat TomSweeneyRedHat force-pushed the dev/tsweeney/moveversions branch 2 times, most recently from 2df0dcf to 3a14eb1 Compare May 21, 2026 18:41
Change the version file for c/common and c/storage to emulate
the version file that is in c/image.  This updates version/version.go in
c/common, and moves the VERSION file in c/storage to version/version.go.

Signed-off-by: Tom Sweeney <tsweeney@redhat.com>
@TomSweeneyRedHat TomSweeneyRedHat force-pushed the dev/tsweeney/moveversions branch from 3a14eb1 to 968ac67 Compare May 21, 2026 20:13
Copy link
Copy Markdown
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

ACK, but tests are failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to "common" package storage Related to "storage" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants