contributte / logging

:boom: Universal logging support to Tracy / Nette Framework (@nette)
https://contributte.org/packages/contributte/logging.html
MIT License
20 stars 18 forks source link

Sentry: Add enabled config option #11

Closed RiKap closed 6 years ago

RiKap commented 6 years ago

Add easy way to disable Sentry logging

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 44.444% when pulling 0ef348492cb99ffeb9294099b841c7b006fab110 on RiKap:patch-2 into 67a02b3d1105c09e36f1041e7b90ea3ec3a36f50 on contributte:master.

RiKap commented 6 years ago

@f3l1x What do you think?

f3l1x commented 6 years ago

I get the point. Isn't better to just not register the SentryLogger at all?

RiKap commented 6 years ago

@f3l1x I don't think so. I just register all loggers in config.neon. When I put code on testing instance I do not need logs log into sentry. So I think that it is much easier to have, in config.local.neon this:

sentry:
    enable: true/false

than this:

extensions:
    sentry: Contributte\Logging\DI\SentryLoggingExtension
        url: '.....'

I see that is almost same, but config.local.neon I want to change only values not register new extensions, services ....

f3l1x commented 6 years ago

I ment don't register SentryLogger in DI. So your code will be same except for enabled: true/false.

RiKap commented 6 years ago

@f3l1x When I register SentryLoggingExtension it automatically register SentryLogger. Yes I can skip registration of sentry extension and use logging.loggers configuration to just add SentryLogger in config.local.neon. But I still think that enabled: true/false is better than register SentryLogger or register anything in config.local.neon.

Anyway it is up to you. It is not dealbreaker for me, just nice thing to have. :)

RiKap commented 6 years ago

@f3l1x Ahhh, sorry. I thought that you talk about configuration, not PR. PR is updated.

f3l1x commented 6 years ago

Cool, thanks.

f3l1x commented 6 years ago

👍