-
Notifications
You must be signed in to change notification settings - Fork 112
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
Remove Windows Support #1441
base: main
Are you sure you want to change the base?
Remove Windows Support #1441
Conversation
I took a swing at removing any and all references to windows in this reference implementation. Lifecycle no longer outputs a windows binary as a release artifact Lifecycle no longer operates against windows containers buildpacks/rfcs#311 https://medium.com/buildpacks/deprecation-announcement-windows-container-feature-in-cloud-native-buildpacks-bbb70351343d Signed-off-by: Jesse Brown <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit about test contexts but lgtm
Signed-off-by: Jesse Brown <[email protected]>
@@ -486,7 +486,7 @@ func testAnalyzer(platformAPI string) func(t *testing.T, when spec.G, it spec.S) | |||
|
|||
it("populates target metadata from the run image", func() { | |||
h.AssertNil(t, previousImage.SetLabel("io.buildpacks.base.id", "id software")) | |||
h.AssertNil(t, previousImage.SetOS("windows")) | |||
h.AssertNil(t, previousImage.SetOS("zindows")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the deal with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to rid ourselves of any references to the string windows
- I mostly did it as I removed all references but decided to leave it so it was clear we don't actually support windows OS images in this repo.
@@ -744,7 +744,7 @@ func testDetector(t *testing.T, when spec.G, it spec.S) { | |||
Targets: []buildpack.TargetMetadata{ | |||
{Arch: "P6", ArchVariant: "Pentium Pro", OS: "Win95", | |||
Distros: []buildpack.OSDistro{ | |||
{Name: "Windows 95", Version: "OSR1"}, {Name: "Windows 95", Version: "OSR2.5"}}}, | |||
{Name: "Sceens 95", Version: "OSR1"}, {Name: "Sceens 95", Version: "OSR2.5"}}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O_O
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as #1441 (comment)
Summary
I took a swing at removing any and all references to windows in this reference implementation. There are for sure areas that can be cleaned up and reduce complexity further. This was a first pass to get things compiling and actions working.
Release notes
Windows support has been removed from lifecycle. Lifecycle no longer outputs a windows binary as a release artifact Lifecycle no longer operates against windows containers.
Related
buildpacks/rfcs#311
Context
https://medium.com/buildpacks/deprecation-announcement-windows-container-feature-in-cloud-native-buildpacks-bbb70351343d