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

Implements Proposal: Address Base Image Reproducibility #245

Merged
merged 12 commits into from
May 3, 2018

Conversation

coollog
Copy link
Contributor

@coollog coollog commented May 2, 2018

LayerBuilder layerBuilder =
new LayerBuilder(
sourceFiles, extractionPath, buildConfiguration.getEnableReproducibleBuilds());
LayerBuilder layerBuilder = new LayerBuilder(sourceFiles, extractionPath, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to keep this last parameter/reproducibility logic in LayerBuilder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept this here since the LayerBuilder is low-level logic, so the trade-off of removing the explicit "true" here would be a comment added to LayerBuilder saying that it only builds reproducibly.

Copy link
Contributor

@TadCordle TadCordle May 2, 2018

Choose a reason for hiding this comment

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

I think given that we're taking an "always reproducible" approach with jib now, it's implied that LayerBuilder will make reproducible layers, so I don't think we'd need to explicitly comment on that. Comment aside, I just don't think there's much use to keeping the internal enableReproducibleBuilds logic around if it isn't going to be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option could be to rename LayerBuilder to ReproducibleLayerBuilder, if you want to be explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ReproducibleLayerBuilder sounds like a good idea

@loosebazooka
Copy link
Member

loosebazooka commented May 3, 2018

Should we move the proposal to "archives" or something as part of this PR?

@coollog
Copy link
Contributor Author

coollog commented May 3, 2018

Yeah "archives" sounds like a good idea.

@coollog coollog merged commit 9a016d2 into master May 3, 2018
@coollog coollog deleted the reproducible-base-image branch May 3, 2018 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants