pkg/pwalk: deprecate in favor of pkg/pwalkdir #185
Conversation
thaJeztah
requested review from
kolyshkin,
mrunalp,
rhatdan and
runcom
as code owners
|
@kolyshkin @mrunalp @rhatdan PTAL |
pkg/pwalk/pwalk.go
Outdated
| return walkN(root, walkFn, num) | ||
| } | ||
|
|
||
| func walkN(root string, walkFn WalkFunc, num int) error { |
There was a problem hiding this comment.
What is the meaning of this change? Is that for one deprecated function to not use another? In this case perhaps it's easier to annotate instead.
There was a problem hiding this comment.
Yeah, both my IDE and linter were complaining about using deprecated functions; so could've either added a //nolint:<xxx> flag, or do this.
I'm wondering though if we perhaps should immediately proceed with aliasing this to the new package, if we're going to support go1.16+ (#183), WDYT?
There was a problem hiding this comment.
I changed this back.
w.r.t. aliasing; we can't alias it as the signatures don't match, so let's deprecate it as step 1, then remove after
thaJeztah
force-pushed
the
deprecate_pkg_pwalk
branch
from
80fdf9e to
5b239e6
Compare
|
@kolyshkin updated; PTAL |
|
LGTM |
|
@rhatdan you need to use GitHub's "approve" now (we're no longer using pullapprove on this repo 😁) |
|
/approve |
The package was already deprecated in the README, but not in code. This patch marks the code as deprecated, as go1.15 reached EOL, so all consumers should be able to use the new package. In a follow-up, we remove it (after it's been in a release to allow users to migrate). Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
deprecate_pkg_pwalk
branch
from
5b239e6 to
de26d39
Compare
|
@kolyshkin @giuseppe PTAL |
| @@ -29,6 +33,8 @@ type WalkFunc = filepath.WalkFunc | |||
| // - if more than one walkFn instance will return an error, only one | |||
| // of such errors will be propagated and returned by Walk, others | |||
| // will be silently discarded. | |||
| // | |||
| // Deprecated: use [github.com/opencontainers/selinux/pkg/pwalkdir.Walk] | |||
There was a problem hiding this comment.
Kudos for using doc links!
Nit: missing period at EOL.
There was a problem hiding this comment.
Let me try it again locally, but I think there may have been a bug in Golang's parser; the doc links only work when surrounded by whitespace (to prevent map[string]string being seen as a link), but that also meant that adding a period at the end cause it not to be seen as a link 😞
(Haven't filed an issue for it yet, but did for another one; golang/go#56072)
| @@ -38,6 +44,8 @@ func Walk(root string, walkFn WalkFunc) error { | |||
| // num walkFn will be called at any one time. | |||
| // | |||
| // Please see Walk documentation for caveats of using this function. | |||
| // | |||
| // Deprecated: use [github.com/opencontainers/selinux/pkg/pwalkdir.WalkN] | |||
There was a problem hiding this comment.
Nit: missing period at EOL.
The package was already deprecated in the README, but not in code. This patch marks the code as deprecated, as go1.15 reached EOL, so all consumers should be able to use the new package.
In a follow-up, we can make these functions an alias for the new package (or remove it altogether).