Skip to content

Removing old custom RootFS behavior on Windows. #25090

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
Merged
LK4D4 merged 1 commit into from
Aug 5, 2016
Merged

Removing old custom RootFS behavior on Windows. #25090

LK4D4 merged 1 commit into from
Aug 5, 2016

Conversation

@swernli
Copy link
Contributor

@swernli swernli commented Jul 26, 2016

@jhowardmsft @jstarks
Windows base layers are no longer the special "layers+base" type, so we can remove all the special handling for that.

Signed-off-by: Stefan J. Wernli [email protected]

@lowenna
Copy link
Member

lowenna commented Jul 26, 2016

Nice! LGTM once Janky passes. Has some import fixups required:

14:10:12 distribution/pull_v2_unix.go:8: imported and not used: "github.com/docker/distribution/manifest/schema1"
14:10:12 distribution/pull_v2_unix.go:9: imported and not used: "github.com/docker/docker/image"

@thaJeztah thaJeztah added this to the 1.13.0 milestone Jul 26, 2016
@jstarks
Copy link
Contributor

jstarks commented Jul 27, 2016 •
edited
Loading

LGTM

@thaJeztah
Copy link
Member

thaJeztah commented Jul 29, 2016

LGTM, but

ping @aaronlehmann @stevvooe for the change in image-specs

aaronlehmann
aaronlehmann reviewed Jul 29, 2016
View reviewed changes
daemon/oci_windows.go Outdated
Copy link

@aaronlehmann aaronlehmann Jul 29, 2016

Choose a reason for hiding this comment

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

nit: extra parentheses

@aaronlehmann
Copy link

aaronlehmann commented Jul 29, 2016

Seems fine to me. The base layer stuff was hack that we didn't want to support long-term. It's great that we're able to remove it.

Normally there would be backward compatibility concerns, but I know the Windows daemon is not in wide use and there are not many images for it, so if you feel it's okay to get rid of this as a step change, that's fine with me.

Needs a rebase though.

cc @dmcgowan @tonistiigi

stevvooe
stevvooe reviewed Jul 29, 2016
View reviewed changes
image/spec/v1.1.md Outdated
Copy link
Contributor

@stevvooe stevvooe Jul 29, 2016

Choose a reason for hiding this comment

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

We should probably leave this in or omit from OCI if we don't intend to support it going forward.

Copy link
Contributor Author

@swernli swernli Jul 29, 2016

Choose a reason for hiding this comment

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

Good point. I'll file an issue against @jstarks to get this removed from OCI.

Copy link
Contributor Author

@swernli swernli Jul 29, 2016

Choose a reason for hiding this comment

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

Actually, looks like you already opened that issue!

@swernli
Removing old custom RootFS behavior on Windows.
f342b27 
Windows base layers are no longer the special "layers+base" type, so we can remove all the special handling for that.

Signed-off-by: Stefan J. Wernli <[email protected]>
@LK4D4
Copy link
Contributor

LK4D4 commented Aug 5, 2016

I think it has plenty of lgtms

@LK4D4 LK4D4 merged commit 6c07fbd into moby:master Aug 5, 2016
@wking wking mentioned this pull request Sep 1, 2016
Closed
@lowenna lowenna mentioned this pull request Sep 8, 2016
Merged
@thaJeztah thaJeztah mentioned this pull request Feb 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area/distribution Image Distribution platform/windows status/2-code-review

Projects

None yet

Milestone

1.13.0

Development

Successfully merging this pull request may close these issues.

8 participants

@swernli @lowenna @jstarks @thaJeztah @aaronlehmann @LK4D4 @stevvooe @GordonTheTurtle