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.

Sign up for GitHub

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

Jump to bottom

chore: refactor for handling of cgroup driver #876

Merged
merged 2 commits into from
Feb 26, 2025
Merged

chore: refactor for handling of cgroup driver #876

merged 2 commits into from
Feb 26, 2025

Conversation

z63d
Copy link
Contributor

@z63d z63d commented Feb 25, 2025 •
edited
Loading

Refactor of #864 .

@z63d z63d marked this pull request as draft February 25, 2025 13:39
@z63d z63d force-pushed the feat/refactor-cgroup-driver branch 2 times, most recently from 40fe3ac to 31e1d9f Compare February 25, 2025 14:51
@Mossaka Mossaka mentioned this pull request Feb 25, 2025
Closed
jprendes
jprendes reviewed Feb 26, 2025
View reviewed changes
Copy link
Collaborator

@jprendes jprendes left a comment

Choose a reason for hiding this comment

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

Thanks!
This looks good. Left a few nits.

@z63d, are you planning to add the tests in this PR or in a new one?

@z63d
refactor: runtime options config
99b19df 
Signed-off-by: Kaita Nakamura <kaita.nakamura0830@gmail.com>
@z63d z63d force-pushed the feat/refactor-cgroup-driver branch from 31e1d9f to 99b19df Compare February 26, 2025 11:04
@z63d
Copy link
Contributor Author

z63d commented Feb 26, 2025 •
edited
Loading

@jprendes
I was going to add the test commits to this PR. Of course it can be split.
Split?

I thought it would be better to split the Options struct and Config struct into different files(e.g. runtimeoptions.rs), but what are your thoughts on this?

@z63d z63d marked this pull request as ready for review February 26, 2025 12:19
jprendes
jprendes previously approved these changes Feb 26, 2025
View reviewed changes
Copy link
Collaborator

@jprendes jprendes left a comment

Choose a reason for hiding this comment

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

Thanks!
LGTM

@jprendes
Copy link
Collaborator

Split?

Yeah, that'd be best :-)

@jprendes
Update crates/containerd-shim-wasm/src/sandbox/instance.rs
36e0411 
Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>
@z63d z63d changed the title feat: refactor/add-tests for handling of cgroup driver chore: refactor for handling of cgroup driver Feb 26, 2025
@jprendes jprendes merged commit 3240088 into containerd:main Feb 26, 2025
77 checks passed
@z63d z63d deleted the feat/refactor-cgroup-driver branch February 26, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jprendes jprendes jprendes approved these changes

Assignees
No one assigned
Labels
C-containerd-shim-wasm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@z63d @jprendes