-
Notifications
You must be signed in to change notification settings - Fork 10
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
Multi arch support #357
Multi arch support #357
Conversation
ace5290
to
957976e
Compare
957976e
to
2fad871
Compare
@ForestEckhardt I updated freezer, rebased branch. Is ready for review :) |
LGTM, can you rebase the branch and check if the changes to |
2fad871
to
cb8fb1b
Compare
feat: multi-arch support for generating multi-arch directories
cb8fb1b
to
466a877
Compare
@modulo11 thank you, i removed them and the tests are passing |
@ForestEckhardt I think this PR is ready for review/merge |
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.
LGTM
@@ -29,16 +29,15 @@ func testBuildpackStore(t *testing.T, when spec.G, it spec.S) { | |||
fakeCacheManager = &fakes.CacheManager{} | |||
|
|||
buildpackStore = occam.NewBuildpackStore() | |||
|
|||
buildpackStore = buildpackStore.WithLocalFetcher(fakeLocalFetcher). |
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 don't think we needed to remove this - we could have kept it here, and later on we could have done buildpackStore = buildpackStore.WithTarget()
. It would only override it for the specific tests that require it.
However, i think either option is fine, so no need to revert your changes.
Merge After
https://github.com/ForestEckhardt/freezer
package.Summary
This PR does two things:
WithTargets
which allows to download the proper buildpack architecture that is specified from that flag. If not specified, default value islinux/amd64
.Use Cases
Checklist