pkg/pwalk: deprecate in favor of pkg/pwalkdir#185
pkg/pwalk: deprecate in favor of pkg/pwalkdir#185kolyshkin merged 1 commit intoopencontainers:mainfrom
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 |
| // 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)
| // | ||
| // 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.
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
A couple of nits, but since this package is to be removed soon anyway, it doesn't matter much.
LGTM
3 participants
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).