Updating minimum Go version to 1.19 #3304
Conversation
iwankgb
force-pushed
the
go-1.19
branch
3 times, most recently
from
d6277f9 to
6078378
Compare
iwankgb
force-pushed
the
go-1.19
branch
3 times, most recently
from
bc2f0ba to
f4c2421
Compare
container/docker/handler_test.go
Outdated
| testDir, err := ioutil.TempDir("", "") | ||
| testDir, err := os.MkdirTemp(os.TempDir(), "") |
There was a problem hiding this comment.
it's better to use t.TemDir() here.
fs/fs_test.go
Outdated
| fsInfo, err := NewFsInfo(Context{}) | ||
| as.NoError(err) | ||
| dir, err := ioutil.TempDir(os.TempDir(), "") | ||
| dir, err := os.MkdirTemp(os.TempDir(), "") |
There was a problem hiding this comment.
better to use t.TempDir() here.
| func (fs *realSysFs) GetCaches(id int) ([]os.FileInfo, error) { | ||
| cpuPath := fmt.Sprintf("%s%d/cache", cacheDir, id) | ||
| return ioutil.ReadDir(cpuPath) | ||
| dir, err := os.ReadDir(cpuPath) |
There was a problem hiding this comment.
I know this is an incompatible change, but it would make sense to convert this to return []os.DirEntry instead.
Otherwise, practically, what happens here, is a stat(2) call to every entry, which is kinda expensive. More to say, it seems the only user of this function is in utils/sysinfo (I checked), so while this is a public API, apparently it has no users.
There was a problem hiding this comment.
There are three other uses of toFileInfo() that would have to be fixed. While I understand you concern and I believe that it's valid, I'm hesitant to do this as part of this PR and I would prefer to address it in future.
|
/assign @bobbypage |
Creatone
left a comment
Creatone
left a comment
There was a problem hiding this comment.
LGTM
Will you handle the @kolyshkin proposal on the t.TempDir?
|
I did in 814bb27 |
I miss that, sorry |
|
delayed lgtm, thank you for the fixes! |
4 participants
Reasoning behind this proposal:
1.20onmainanyway1.19and1.20only/and/cmdshall be the same.Note for the reviewers: take a look at
utils.sysfs.toFileInfo()- this the only addition/change that was not performed using replace functionality in my IDE.