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

libct: rm BaseContainer and Container interfaces #3375

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Feb 14, 2022

Based on and currently includes #3373. Draft until that one is merged.

The only implementation of BaseContainer and Container is linuxContainer. It does not make
sense to have an interface with a single implementation, and we do not
foresee other types of containers being added to runc.

  • Remove BaseContainer andContainer interfaces, moving their methods
    documentation to linuxContainer.

  • Rename linuxContainer to Container.

  • Adopt users from using interface to using struct.

Continuing the spring cleaning started in #3354. Loosely based on parts of #3316.

@kolyshkin kolyshkin mentioned this pull request Feb 14, 2022
7 tasks
@kolyshkin kolyshkin mentioned this pull request Feb 19, 2022
@kolyshkin kolyshkin force-pushed the rm-container-iface branch from 99333b4 to 569b7c0 Compare March 10, 2022 03:11
@thaJeztah
Copy link
Member

@kolyshkin this one can be rebased (was considering "we should remove those interfaces", then recalled: "we had a PR for that") 😂

The only implementation of these is linuxContainer. It does not make
sense to have an interface with a single implementation, and we do not
foresee other types of containers being added to runc.

Remove BaseContainer and Container interfaces, moving their methods
documentation to linuxContainer.

Rename linuxContainer to Container.

Adopt users from using interface to using struct.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin force-pushed the rm-container-iface branch from 569b7c0 to 102b8ab Compare March 23, 2022 18:04
@kolyshkin kolyshkin marked this pull request as ready for review March 23, 2022 18:05
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Mar 23, 2022

@kolyshkin this one can be rebased (was considering "we should remove those interfaces", then recalled: "we had a PR for that") 😂

Rebased; no longer a draft; PTAL @thaJeztah @AkihiroSuda 🙏🏻

(I still have one more PR on top of this, #3385)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM!

@thaJeztah thaJeztah merged commit 86d6898 into opencontainers:main Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants