remove some (aliases for) deprecated functions#7525
Conversation
thaJeztah
changed the title
thaJeztah
force-pushed
the
remove_deprecated_stubs
branch
from
97dd82a to
598b606
Compare
|
|
||
| base, options, close := tlsServer(wrapped) | ||
| authorizer := NewDockerAuthorizer( | ||
| WithAuthClient(options.Client), |
There was a problem hiding this comment.
Note that options.Client is also deprecated, in favour of .. erm. options.Host (which is what we're creating here) 😂)
thaJeztah
force-pushed
the
remove_deprecated_stubs
branch
2 times, most recently
from
434a005 to
8086af4
Compare
|
Makes me wonder, do we have a general removal policy for deprecated code (e.g. X releases after being deprecated this is safe to remove)? Some of these definitely seem overdue, the userns one being the most recent as it was in 1.5 so the X would be 2 in that case which seems fine to me |
The alias is in the 1.6 release, which should give consumers time to migrate. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Has been deprecated in containerd v1.3.0, so we can remove this. Signed-off-by: Sebastiaan van Stijn <[email protected]>
These have been deprecated since containerd v1.4.0. Signed-off-by: Sebastiaan van Stijn <[email protected]>
This was deprecated since containerd v1.3.0. Signed-off-by: Sebastiaan van Stijn <[email protected]>
This was no longer used since 058eea3 (v1.0.0-alpha0), and there's no external users. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
remove_deprecated_stubs
branch
from
8086af4 to
6142a2a
Compare
|
@dcantah good question, and perhaps something we should mention explicitly. In general, I think it's acceptable (and common practice) to;
For most consumers of the module this would mean that get a notice (deprecation) when updating the module, and a reasonable time window to update their code (especially now that we have LTS releases). While perhaps ideally such things would only be done in major releases, time between major releases may be quite long, and with go modules, CRI API versions and the containerd project itself "competing over" who "owns" the SemVer version, this may tie our hands too much. Admitted, for some of these aliases, maintenance cost isn't very high, but my experience is that keeping the aliases longer won't help with those that ignore deprecations. Of course this should be done within reason (e.g., we can't deprecate CRI and remove it in a non-major release). |
5 participants
sys: remove alias for deprecated sys.RunningInUserNS()
The alias is in the 1.6 release, which should give consumers time
to migrate.
pkg/cri/util/: remove deprecated NormalizeImageRef alias
Has been deprecated in containerd v1.3.0, so we can remove this.
sys: remove aliases for deprecated EpollCreate1, EpollCtl, EpollWait
These have been deprecated since containerd v1.4.0.
remotes/docker: remove deprecated NewAuthorizer alias
This was deprecated since containerd v1.3.0.
sys: remove unused GetOpenFds()
This was no longer used since 058eea3
(v1.0.0-alpha0), and there's no external users.