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

Switch most calls to filepath.Walk to filepath.WalkDir #1176

Merged
merged 2 commits into from Apr 6, 2022

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Mar 26, 2022

It is faster then Walk, when you don't need to stat every
file and directory.

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

@rhatdan rhatdan force-pushed the main branch 4 times, most recently from ad0d0f2 to 5c036e5 Compare March 28, 2022 12:53
@rhatdan
Copy link
Member Author

rhatdan commented Mar 28, 2022

@nalind @cevich This looks like the images does not have a newer golang?

@cevich
Copy link
Member

cevich commented Mar 28, 2022

This looks like the images does not have a newer golang?

Newer golang is F36 only and in containers/podman#13376

@nalind
Copy link
Member

nalind commented Mar 28, 2022

The job description for https://cirrus-ci.com/task/4664820296318976?logs=setup#L176 says it's Fedora 34, but for whatever reason it's Fedora 33. The standard library introduced the io/fs package in 1.16, which was included in Fedora 34.

@rhatdan rhatdan force-pushed the main branch 2 times, most recently from a1f11ee to 40be7e6 Compare March 29, 2022 10:59
@rhatdan
Copy link
Member Author

rhatdan commented Mar 29, 2022

@cevich could you update the images for storage. Podman's images do not want. But I need newer versions of golang on f34.

@cevich
Copy link
Member

cevich commented Mar 29, 2022

could you update the images for storage

Yep.

@cevich
Copy link
Member

cevich commented Mar 29, 2022

Let's see what #1182 do.

@TomSweeneyRedHat
Copy link
Member

All kinds of test unhappiness @rhatdan

@rhatdan rhatdan force-pushed the main branch 2 times, most recently from 0e6a4b3 to 43c6eef Compare March 30, 2022 12:59
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@rhatdan
Copy link
Member Author

rhatdan commented Mar 30, 2022

Yup I am at a loss to why this works on Podman but not in storage.

@rhatdan rhatdan force-pushed the main branch 2 times, most recently from 5c4dc4c to 9327f72 Compare April 1, 2022 22:09
@TomSweeneyRedHat
Copy link
Member

Changes LGTM
lint is complaining about not being able to find a package, plus you need to rebase, apparently.

@nalind
Copy link
Member

nalind commented Apr 5, 2022

The lint task in the Cirrus config specifies that it runs in golang:1.15 on line 120 of .cirrus.yml, and io/fs was new in 1.16.

@cevich
Copy link
Member

cevich commented Apr 5, 2022

The lint task in the Cirrus config specifies that it runs in golang:1.15 on line 120 of .cirrus.yml, and io/fs was new in 1.16.

IIUC, it's totally fine to just bump that up in the YAML, shouldn't affect anything other than that one lint task.

@rhatdan rhatdan force-pushed the main branch 2 times, most recently from 3b383f2 to 58d6217 Compare April 6, 2022 12:50
It is faster then Walk, when you don't need to stat every
file and directory.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@rhatdan rhatdan merged commit 296e6aa into containers:main Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants