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

Use newer x/sys/windows SecurityAttributes struct (carry 40017) #40021

Merged
merged 2 commits into from Nov 21, 2019

Conversation

@thaJeztah
Copy link
Member

thaJeztah commented Sep 30, 2019

carries #40017
closes #40017

changes since #40017:

This struct now has a properly typed member, so use the properly typed functions with it.

@zx2c4

This comment has been minimized.

Copy link
Contributor

zx2c4 commented Oct 1, 2019

LGTM. 👍

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 1, 2019

@tklauser

This comment has been minimized.

Copy link
Contributor

tklauser commented Oct 1, 2019

LGTM, but I'd wait for the outcome of the discussion on golang/go#34610 where the decision might be to revert the change to SecurityAttributes and introduce a new struct type with a different name.

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 1, 2019

Thanks @tklauser - let me mark this PR as "WIP" in the meantime. I'll also comment on the PR in Microsoft/go-winio

@thaJeztah thaJeztah changed the title Use newer x/sys/windows SecurityAttributes struct (carry 40017) [WIP] Use newer x/sys/windows SecurityAttributes struct (carry 40017) Oct 1, 2019
thaJeztah and others added 2 commits Sep 30, 2019
full diff: golang/sys@9eafafc...c990c68

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This struct now has a properly typed member, so use the properly typed
functions with it.

Also update the vendor directory and hope nothing explodes.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the thaJeztah:carry_40017 branch from b7b9e08 to c3a0a37 Oct 2, 2019
@thaJeztah thaJeztah changed the title [WIP] Use newer x/sys/windows SecurityAttributes struct (carry 40017) [do not merge] Use newer x/sys/windows SecurityAttributes struct (carry 40017) Oct 3, 2019
@ArmandGrillet

This comment has been minimized.

Copy link

ArmandGrillet commented Nov 5, 2019

According to golang/go#34610 (comment) and the status of the issue (i.e. stale), the change will not be reverted.

@tklauser

This comment has been minimized.

Copy link
Contributor

tklauser commented Nov 8, 2019

@thaJeztah This is good to go now that golang/go#34610 has been closed and it was decided there will be no revert (golang/go#34610 (comment))

@thaJeztah thaJeztah changed the title [do not merge] Use newer x/sys/windows SecurityAttributes struct (carry 40017) Use newer x/sys/windows SecurityAttributes struct (carry 40017) Nov 8, 2019
@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Nov 8, 2019

@ArmandGrillet @tklauser thanks for the heads-up; I removed "do not merge" from the title, and triggered CI

@ArmandGrillet

This comment has been minimized.

Copy link

ArmandGrillet commented Nov 21, 2019

CI looks green 🎉, @tonistiigi can you please review this PR?

@tonistiigi tonistiigi merged commit d1d5f64 into moby:master Nov 21, 2019
2 checks passed
2 checks passed
DCO DCO
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
@thaJeztah thaJeztah deleted the thaJeztah:carry_40017 branch Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.