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

[LTB-37] log: Rustic logging for lazy people #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kirelagin
Copy link
Contributor

Stupid logging backend compatible with the loot-log interface but not requiring anything except for MonadIO. Use it in your throw-away tools.

@kirelagin kirelagin changed the title log: Rustic logging for lazy people [LTB-37] log: Rustic logging for lazy people Aug 11, 2018
-- | Down to earth logging for the lazy ones
--
-- No configuration, just use functions from this module instead of @putStrLn@
-- and they will do the rest. All messages above @Debug@ are printed to @stderr@.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather use global variable to control this behaviour. We can present a function setDebug :: IO () that enables debug output by enabling a parameter inside MVar. Is this approach any better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I was thinking about it and I am not sure tbh. My view on this is that having a global variable no longer feels that dumb-simple and pure, while the only benefit it gives is a sort of dynamic configuration, e.g. with a command-line switch, and if you want this, it probably means that it’s time to switch to a proper logging implementation. This one is meant only for the simplest things, one-off scripts, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Compilation flags look scary to me -- of course, they are easy to configure, but I need to look up the exact way to do it. Instead, adding a line of hs code enabling debug output is straightforward. The user doesn't need to know how exactly it is implemented under the bonnet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I really think that for simplest things and one-off scripts it's not the compilation flag which should be used, but an environment variable. If you're quickly prototyping something, you really don't want to recompile the thing to enable some debug logs for the next run.

Copy link
Member

Choose a reason for hiding this comment

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

CPP also looks scary to me, because I need to compile a library with -DDEBUG, not my application, AFAIU. Even if I am wrong, even the need to think about it doesn't sound good.
+1 to environment variable.

@kirelagin
Copy link
Contributor Author

We need more opinions on this 🤔

@volhovm
Copy link
Contributor

volhovm commented Oct 19, 2018

@kirelagin this PR is actually very good, could we move forward? I still insist on adding the option to change parameters from the code directly, because I find both env var and compilation flag to be complex (first forces me to export some flag in every console I launch my program in, the second one is even more difficult).

what the `caps` framework is meant to do. The only thing `loot-log` has to do is provide
access to the configuration and then you will simply use `adjustCap`.
by any code called by a specific function? Hopefully, you’ll be able to do this
somehow, but this functionality is currently in the works.
Copy link
Member

Choose a reason for hiding this comment

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

How is it related to rustic logging?

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

5 participants