Skip to content

Conversation

@mtrmac
Copy link
Contributor

@mtrmac mtrmac commented Sep 4, 2025

… and benefit from the new features.

Also a set somewhat-related cleanups; see individual commit messages for details.

Warning: absolutely untested in practice.

@github-actions github-actions bot added storage Related to "storage" package common Related to "common" package image Related to "image" package labels Sep 4, 2025
@Luap99
Copy link
Member

Luap99 commented Sep 5, 2025

With the current github action we use it should be trivial to add a macos and windows lint job as well, given there is a fair amount of platform specific code that might make sense to do.

Comment on lines 121 to 122
if top, rest, ok := strings.Cut(repo.path, "/"); ok && top == officialRepoName {
repo.path = rest
Copy link
Member

Choose a reason for hiding this comment

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

this is not the same and I guess that is what the tests are failing

strings.Cut() is equal to strings.SplitN(repo.path, "/",2), in the new case you match the with one or more / wile do old one was exact one /

Comment on lines 64 to 65
minorStr, majorStr, ok := strings.Cut(k, ":")
if !ok {
Copy link
Member

Choose a reason for hiding this comment

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

I assume this works the same as the file format only describes minor:major but technically this is not the same.
Previously if is a second : we just continue, now it will be part of majorStr and thus fail in ParseUint

Comment on lines 78 to 79
op, valueStr, ok := strings.Cut(item, "=")
if !ok {
Copy link
Member

Choose a reason for hiding this comment

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

same thing here

@Luap99
Copy link
Member

Luap99 commented Sep 8, 2025

Non blocking but podman CI is not yet ready for 1.24 containers/podman#27017 (comment) but that should be solved soon so we can definitely merge this here.

@mtrmac
Copy link
Contributor Author

mtrmac commented Sep 8, 2025

@Luap99 Thanks, updated (along with a few more reverts of overzealous strings.Cut, and some small cleanups on top).

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM
@containers/container-libs-maintainers PTAL

Unrelated, just to make my work easier.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Don't create single strings only to split them again;
avoid over-abstracted arrays that nevertheless assume
a specific structure.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Passing nil as the unmarshaling destination is completely invalid,
so, don't.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The storage/drivers/aufs/aufs_test.go use is unmodified
because it loops over b.N inside another loop.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
CreateCertificate does that automatically now.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

/lgtm

> go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix ./...

and manually filter out "for var := range $number" which I aesthetically dislike.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
> Go run golang.org/x/tools/internal/gofix/cmd/gofix@latest -fix ./...

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Don't append into a long-lived slice.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac merged commit 71ca11c into containers:main Sep 9, 2025
36 checks passed
@mtrmac mtrmac deleted the go1.24 branch September 9, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants