charkost / prosopite

:mag: Rails N+1 queries auto-detection with zero false positives / false negatives
Apache License 2.0
1.48k stars 45 forks source link

Allow setting a custom logger? #8

Closed F-3r closed 2 years ago

F-3r commented 3 years ago

Hi! I was trying out Prosopite in one of our project and looks really good so far :) congrats!

We don't use the raise option as it would be too disruptive (sadly we have a few old n+1 that will take some time to fix that we don't have) so, I thought that it would be nice to log the errors in RED so no one can say they didn't see them :rofl:

What do you think about adding that option to the config or perhaps, simplify the config to a single logger entry?

I'm currently using this patch which implements the second option: https://github.com/F-3r/prosopite/commit/4db811c4260f89f94369f94783d199a8085945bb

I can open a PR if you think it is worth it.

PS: Also, now that I've been looking into the code, I was wondering if you would be interested in some peer-code-review?

charkost commented 3 years ago

Hello! Thanks for the feedback & suggestions!

I 100% agree for the red color. It should apply only to the stderr + rails loggers.

Regarding the custom logger, i think that this functionality will be good to be available using an extra Prosopite.custom_logger.

A PR will be welcome!

If you find any other issues in the code feel free to open issues / PRs.

F-3r commented 3 years ago

Ahhh I missed this last message and I've already sent the PR. Leave me any comments you want and I'll check them tomorrow!