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

RFC: log to stderr not stdout [might not take this one] #60

Closed
wants to merge 1 commit into from

Conversation

weissi
Copy link
Member

@weissi weissi commented Apr 16, 2019

Motivation:

Logging to stderr makes more sense as it enables command line utilities
to log.

Modifications:

Log to stderr, not stdout

Result:

more useful

Motivation:

Logging to stderr makes more sense as it enables command line utilities
to log.

Modifications:

Log to stderr, not stdout

Result:

more useful
Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

I like the introduction of the two but I'm not sure if stderr should be the default one to print to 🤔

@weissi
Copy link
Member Author

weissi commented Apr 17, 2019

I like the introduction of the two but I'm not sure if stderr should be the default one to print to 🤔

Do you think stdout should be default? What about command line utilities like

cat /tmp/image.gif | my-transcoder > /tmp/image.jpg

?

@ktoso
Copy link
Member

ktoso commented Apr 17, 2019

Yeah that may be a good point... It's one of those things where there's no "best" default I guess though... If you prefer stderr and people agree then should be fine 👍

I also skimmed for some prior art. Results are "it's a mix" 😉 (skimmed java defaults (default stderr, though most popular lib defaults to stdout, configurable; rust has a million mini libs which all do differently, no clear "by default ...").

@weissi
Copy link
Member Author

weissi commented Apr 17, 2019

Yeah that may be a good point... It's one of those things where there's no "best" default I guess though... If you prefer stderr and people agree then should be fine 👍

I also skimmed for some prior art. Results are "it's a mix" 😉 (skimmed java defaults (default stderr, though most popular lib defaults to stdout, configurable; rust has a million mini libs which all do differently, no clear "by default ...").

Hehe, yes, nobody knows. I guess the best argument for stdout is that that's what 1.0.0 does and you could see this as a breaking change. Maybe we'll just leave it as is.

@Yasumoto
Copy link
Contributor

I’m a fan of stderr by default as I can pipe it away or can just directly pipe my output to another tool without having to add some kind of grep -v

/// Ships with the logging module, really boring just prints something using the `print` function
internal struct StdoutLogHandler: LogHandler {
internal struct StderrLogHandler: LogHandler {
Copy link
Member

@tomerd tomerd Apr 17, 2019

Choose a reason for hiding this comment

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

=> i suggest we take an enum here to allow the user the choose between stdout and stderr
=> as for default, its a good question!

Copy link
Member Author

Choose a reason for hiding this comment

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

@tomerd sure, could do that. StdioLogHandler(stream: StdioLogHandler.Stream) with enum Stream { case stdout; case stderr }

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I like it. Default: dunno :)

Copy link
Member

Choose a reason for hiding this comment

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

lets keep the default stdout, CLIs can fairly easily switch to stderr with this design

@tomerd
Copy link
Member

tomerd commented Apr 25, 2019

@weissi should we close this in favor of #61?

@weissi
Copy link
Member Author

weissi commented Apr 25, 2019

yes

@weissi weissi closed this Apr 25, 2019
weissi pushed a commit that referenced this pull request May 1, 2019
Expose `StreamLogHandler` to the public which replaces the previously internal `StdoutLogHandlers`. Users can choose now the output stream, while `stdout` is the default.

### Motivation:

As discussed on #51 and #60

### Modifications:

Rename `StdoutLogHandler` to `StreamLogHandler` and provide 2 factory methods `standardOutput(label:)` and `standardError(label:)` for using during bootstrapping.

### Result:

Users who adopt other logging backends during `LoggingSystem` bootstrapping will still be able to use the default log handler to get very basic console output to their choice of `stdout` or `stderr`.

### Example:
```
// Log to both stderr and stdout for good measure
LoggingSystem.bootstrap { 
    MultiplexLogHandler([
        FancyLoggingBackend(label: $0),
        StreamLogHandler.standardError(label: $0), 
        StreamLogHandler.standardOutput(label: $0)
    ])
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants