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

Add debug logger #604

Merged
merged 2 commits into from Oct 20, 2021
Merged

Add debug logger #604

merged 2 commits into from Oct 20, 2021

Conversation

dvejmz
Copy link
Contributor

@dvejmz dvejmz commented Oct 19, 2021

Provide logger implementation that allows writing application logs to a file if DEBUG mode environment variable is enabled.

Closes #592.

Example usage

DEBUG=1 revive

This will write a log file called revive.log created in the current working directory, where all subsequent application logs will be appended.

The implementation is very basic as I thought we could start with something simple and build from there, if necessary. I decided to use an environment variable instead of a command-line flag to make it easier to activate this functionality in CI environments. If people have a strong preference to use the command-line option instead, I'm happy to change it (or allow both).

Another assumption I made was that we'd want to write application logs to a file instead of stdout, so as not to mix up application output with debug information.

Feedback welcome on the above or anything else I may have missed!

Provide logger implementation that allows writing application logs to a file if
`DEBUG` mode environment variable is enabled.

Signed-off-by: David Jimenez <dvejmz@sgfault.com>
Signed-off-by: David Jimenez <dvejmz@sgfault.com>
@chavacava
Copy link
Collaborator

Hi @dvejmz thanks for the PR

I'm personally not sure of the utility of a debug mode for revive. I've wrote some linting rules and a few println are enough.
That said, I like your PR because it is not intrusive (e.g. no new flag)

@dvejmz
Copy link
Contributor Author

dvejmz commented Oct 20, 2021

Yeah, I see where you're coming from with that @chavacava . I think perhaps what this PR actually does is not introduce a "debug" mode per se which alters the behaviour of revive, but rather give devs a simple, self-contained logger that can be dropped into any package they happen to be working on at the time and allow them to trace application state wherever it's needed. I added a call to the log in main.go as an example of how it could be used but for the most part, I think it's up to people to decide how much information they want to record via this new verbose mode and perhaps remove those logging statements before merging back into master if they have no further use for them.

I guess the other nice thing about a logger that can be transparently switched on and off is that if people forgot to remove a debug log.Println statement somewhere before merging in a change, it would be suppressed anyway.

@chavacava chavacava merged commit 8a3653c into mgechev:master Oct 20, 2021
@chavacava
Copy link
Collaborator

Thanks @dvejmz

@doniacld
Copy link
Contributor

Thanks @dvejmz for your solution ! :)

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.

Revive in mode debug
3 participants