Skip to content

pkg/system: replace more uses of "syscall"#40645

Merged
cpuguy83 merged 1 commit intomoby:masterfrom
thaJeztah:replace_more_syscall
Mar 9, 2020
Merged

pkg/system: replace more uses of "syscall"#40645
cpuguy83 merged 1 commit intomoby:masterfrom
thaJeztah:replace_more_syscall

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

follow-up to 069fdc8 (#33399), replacing more uses of the syscall package in favor of their "unix", "windows" equivalents in golang.org/x/sys.

@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Mar 9, 2020
@thaJeztah thaJeztah requested a review from lowenna March 9, 2020 10:53
@thaJeztah thaJeztah force-pushed the replace_more_syscall branch 2 times, most recently from 6836a16 to e7ee18e Compare March 9, 2020 11:57
Copy link
Copy Markdown
Member

@lowenna lowenna left a comment

Choose a reason for hiding this comment

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

LGTM with confirmation of that query

Comment thread pkg/system/filesys_windows.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pretty sure this is OK, but can you confirm no higher level caller in the codebase takes a dependency on a specific error. Also applies in OpenFileSequential

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm, good point; given that we documented MkdirAll above as a "drop in" replacement for os.MkdirAll, perhaps it's too risky. I couldn't find an issue at a glance, but given that it's only a small enhancement, let's revert these

follow-up to 069fdc8, replacing
more uses of the syscall package in favor of their "windows"
equivalents in golang.org/x/sys.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Copy link
Copy Markdown
Member

@lowenna lowenna left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83 cpuguy83 merged commit d3f22ac into moby:master Mar 9, 2020
@thaJeztah thaJeztah deleted the replace_more_syscall branch March 9, 2020 19:21
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants