Skip to content

Add new internal cmd package request struct to remove shimdiag package import#1149

Closed
katiewasnothere wants to merge 1 commit intomicrosoft:masterfrom
katiewasnothere:internal_cmd_request_struct
Closed

Add new internal cmd package request struct to remove shimdiag package import#1149
katiewasnothere wants to merge 1 commit intomicrosoft:masterfrom
katiewasnothere:internal_cmd_request_struct

Conversation

@katiewasnothere
Copy link

This PR was created to remove the shimdiag package import in internal/cmd. This allows us to use the internal/cmd package in more places without worry about import cycles or improper use of service types.

Signed-off-by: Kathryn Baldauf kabaldau@microsoft.com

@katiewasnothere katiewasnothere requested a review from a team as a code owner September 4, 2021 01:28
@dcantah dcantah self-assigned this Sep 8, 2021
"golang.org/x/sys/windows"
)

type CmdProcessRequest struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment here on the reason for this. Right now you can only supply it on some specialty methods that are tailored for running a process in a uvm or on the host.

// ExecInShimHost is a helper function used to execute commands specified in `req` in the shim's
// hosting system.
func ExecInShimHost(ctx context.Context, req *shimdiag.ExecProcessRequest) (int, error) {
func ExecInShimHost(ctx context.Context, req *CmdProcessRequest) (int, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like I'm getting deja-vu on another discussion on this naming, but we should probably remove the Shim from this name, can be another PR. Don't think this package should have any mention of it.

…e import

Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>
@katiewasnothere katiewasnothere force-pushed the internal_cmd_request_struct branch from e62da4f to 2fa9e07 Compare September 8, 2021 23:54
@katiewasnothere
Copy link
Author

katiewasnothere commented Sep 8, 2021

I have no idea how this got closed lol

edit for more details: somehow working with my branch locally the branch was rebased? on what? who knows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants