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

cmd/oci-image-tool: use logrus to ease printing to stdout #14

Open
philips opened this issue Sep 21, 2016 · 7 comments · May be fixed by #46
Open

cmd/oci-image-tool: use logrus to ease printing to stdout #14

philips opened this issue Sep 21, 2016 · 7 comments · May be fixed by #46

Comments

@philips
Copy link
Contributor

philips commented Sep 21, 2016

From @runcom on September 9, 2016 8:46

$SUBJECT - logrus have various and a nicer way of printing logs to stdout with the option to specify the log level (which can then be used to differentiate the validation level in a nicer way).
The current way of embedding stdout in the validation struct isn't as pretty as it would be by having a nicer logger like logrus.

I would also suggest to have a context (not necessary context.Context) object which can be used to embed system stuff like a logger - to be the first argument to function like descriptor.Validate(ctx, ...) so we extract the logger from there and print while validating. (Right now this isn't possible, and the logger should be explicitly passed to Validate changing its definition). @stevvooe wdyt?

@s-urbaniak wdyt?

/cc @vbatts @stevvooe @philips

Copied from original issue: opencontainers/image-spec#288

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

From @s-urbaniak on September 9, 2016 9:15

logrus is pretty accepted in the ecosystem, and structured logging doesn't
sound too bad. So I am +1 for this.

On Fri, Sep 9, 2016 at 10:46 AM Antonio Murdaca [email protected]
wrote:

$SUBJECT - logrus have various and a nicer way of printing logs to stdout
with the option to specify the log level (which can then be used to
differentiate the validation level in a nicer way).
The current way of embedding stdout in the validation struct isn't as
pretty as it would be by having a nicer logger like logrus.

@s-urbaniak https://github.com/s-urbaniak wdyt?

/cc @vbatts https://github.com/vbatts @stevvooe
https://github.com/stevvooe @philips https://github.com/philips


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
opencontainers/image-spec#288, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAW8MJU_JyunMxpIqA9OehjG4VLw933cks5qoRzqgaJpZM4J41Ei
.

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

From @xiekeyang on September 9, 2016 10:1

+1

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

From @runcom on September 9, 2016 10:4

I'll work on this once we reach consensus and figure out all the other issues about errors/warnings when validating an image, I would love to have a table-like document which lists soft and hard requirement for image validation w/o having to go through the whole spec to figure out MUST and SHOULD. Do you guys think it would be nice/useful to have such a document for oci-image-tool?

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

From @xiekeyang on September 9, 2016 10:16

It is deep related to #234
In which we are also discussing logrus.
opencontainers/image-spec#234 (comment)

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

From @s-urbaniak on September 9, 2016 10:25

xfref regarding "how to inject the logger": opencontainers/image-spec#234 (comment)

I am in favor of classical dependency injection as per https://peter.bourgon.org/go-best-practices-2016/#program-design.

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

From @wking on September 9, 2016 15:27

On Fri, Sep 09, 2016 at 03:25:13AM -0700, Sergiusz Urbaniak wrote:

I am in favor of classical dependency injection as per
https://peter.bourgon.org/go-best-practices-2016/#program-design.

I'm ok with including a logger in our types to avoid relying on global
state. But transitioning to logrus could happen in two stages:

  • Replace log calls with logrus calls using the global state.
  • Replace global-state logging with explicit logger state carried in
    our types.

Both of those can happen independently. The first should be
straightforward and can happen at any time. The latter should
probably happen along with or after we reorganize validation, etc., to
be more library-like.

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

From @runcom on September 9, 2016 15:29

The latter should
probably happen along with or after we reorganize validation, etc., to
be more library-like.

totally agree on this point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant