Skip to content

Refactor the volume flagging process#1795

Merged
AkihiroSuda merged 1 commit into
containerd:mainfrom
miles170:issue-1680-refactor-volume
Jan 9, 2023
Merged

Refactor the volume flagging process#1795
AkihiroSuda merged 1 commit into
containerd:mainfrom
miles170:issue-1680-refactor-volume

Conversation

@miles170
Copy link
Copy Markdown
Contributor

@miles170 miles170 commented Jan 3, 2023

See #1680

  • Create a file in pkg/api/types/${cmd}.go, and define the CommandOption for this command
  • Create some file in pkg/cmd/${cmd}, and move the command entry point in real into this package

@miles170 miles170 force-pushed the issue-1680-refactor-volume branch 2 times, most recently from 71df3b2 to dccaec2 Compare January 3, 2023 03:30
AkihiroSuda
AkihiroSuda reviewed Jan 3, 2023
View reviewed changes
Comment thread cmd/nerdctl/completion.go Outdated
AkihiroSuda
AkihiroSuda reviewed Jan 3, 2023
View reviewed changes
Comment thread pkg/api/types/volume_types.go Outdated
@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Jan 3, 2023

I think we can retain getVolumes() to avoid code clone

@miles170
Copy link
Copy Markdown
Contributor Author

miles170 commented Jan 3, 2023

I think we can retain getVolumes() to avoid code clone

Keep also getVolumeStore()?

@miles170 miles170 force-pushed the issue-1680-refactor-volume branch 2 times, most recently from 25d403b to 17f82d5 Compare January 4, 2023 06:02
@miles170 miles170 requested a review from AkihiroSuda January 6, 2023 06:12
@miles170 miles170 force-pushed the issue-1680-refactor-volume branch from 17f82d5 to df98d96 Compare January 8, 2023 06:46
@miles170
Copy link
Copy Markdown
Contributor Author

miles170 commented Jan 8, 2023

Force-pushed to resolve code conflicts.

AkihiroSuda
AkihiroSuda approved these changes Jan 8, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda requested a review from Zheaoli January 8, 2023 10:56
Zheaoli
Zheaoli requested changes Jan 8, 2023
View reviewed changes
Comment thread cmd/nerdctl/volume_create.go Outdated
Comment thread cmd/nerdctl/volume_inspect.go Outdated
Comment thread cmd/nerdctl/volume_ls.go Outdated
Comment thread cmd/nerdctl/volume_prune.go Outdated
Comment thread cmd/nerdctl/volume_rm.go Outdated
Comment thread pkg/api/types/volume_types.go Outdated
Comment thread pkg/api/types/volume_types.go Outdated
Comment thread pkg/api/types/volume_types.go Outdated
miles170
miles170 commented Jan 9, 2023
View reviewed changes
Comment thread pkg/api/types/volume_types.go Outdated
@miles170
Refactor the volume flagging process
0c5328c 
Signed-off-by: Miles Liu <miles@bung.cc>
@miles170 miles170 force-pushed the issue-1680-refactor-volume branch from df98d96 to 0c5328c Compare January 9, 2023 05:37
Zheaoli
Zheaoli approved these changes Jan 9, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Zheaoli Zheaoli 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 on CI green!

@djdongjin djdongjin mentioned this pull request Jan 9, 2023
Merged
2 tasks
AkihiroSuda
AkihiroSuda approved these changes Jan 9, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 8c1a6ab into containerd:main Jan 9, 2023
@AkihiroSuda AkihiroSuda added this to the v1.2.0 milestone Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@djdongjin djdongjin djdongjin left review comments

@Zheaoli Zheaoli Zheaoli approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

kind/refactor

Projects

None yet

Milestone

v1.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@miles170 @AkihiroSuda @Zheaoli @djdongjin