Skip to content
This repository was archived by the owner on Jun 1, 2021. It is now read-only.

refactor(): rework of ocibuilder library and cli#65

Merged
artbegolli merged 242 commits intomasterfrom
refactor/buildah-cmd-interface
Dec 2, 2019
Merged

refactor(): rework of ocibuilder library and cli#65
artbegolli merged 242 commits intomasterfrom
refactor/buildah-cmd-interface

Conversation

@artbegolli
Copy link
Collaborator

@artbegolli artbegolli commented Nov 7, 2019

  • Replaces io.ReadCloser output with channels.
  • Client for Buildah and Docker
  • Single OCIBuilder builder.go logic which drives the ocibuilder
  • All interface with the same client
  • Command builder for Buildah exec

@artbegolli artbegolli marked this pull request as ready for review November 20, 2019 16:09
@artbegolli artbegolli self-assigned this Nov 20, 2019
@artbegolli
Copy link
Collaborator Author

artbegolli commented Nov 20, 2019

This is ready to review - ton of changes in this one, restructures a bunch of the codebase

Ignore the number of commits - these came in from the pipeline branch.


stdout, stderr, err := execute(&cmd)
if err != nil {
cli.Logger.WithError(err).Errorln("error building image...")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wrap the error error.Wrap() instead of logging the error all the time

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One note on this - error.Wrap() works but the format of the new error is hard to ready and looks like is just concatenated. I'd prefer to continue logging but only when useful fields or additional information is being added to the logs

What are your thoughts?

@artbegolli artbegolli merged commit 7aa05f8 into master Dec 2, 2019
@artbegolli artbegolli deleted the refactor/buildah-cmd-interface branch December 12, 2019 10:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants