SourceHorizon / logger

Small, easy to use and extensible logger which prints beautiful logs.
https://pub.dev/packages/logger
MIT License
197 stars 33 forks source link

Add Logger with success message [ green color ] 🚀 #37

Closed AbdAlftahSalem closed 1 year ago

AbdAlftahSalem commented 1 year ago

Add Show success response with green color and add an example in the main method 🚀

This RP solve issue number (#12)

Bungeefan commented 1 year ago

Thanks for your PR, now I am able to understand the original issue!

However, I am not really a fan of adding custom log levels to this library, in fact I am currently working on migrating to more common level names like fatal instead of wtf and so on. Maybe it is enough to use a custom printer with such a success level feature.

Are you working on this because you want this solution too, or just to fix it for the original issue author? I am asking, to know with whom I should discuss this further.

AbdAlftahSalem commented 1 year ago

In many cases, the screen needs more than one request to get data from API or needs to get some configuration before starting the app, and we need to show where the specific error is more easily to catch this error in debug mode to show a valid message in release mode and info logger is not enough for me at least. so I want to show success response and show error response maybe it's a good extension in the package to use for all users if want it.

Bungeefan commented 1 year ago

In this case, I would recommend a custom filter, which can help you filter out the noise to identify your problem more easily.

However, I still don't think that such a use case justifies a custom log level in the upstream repository (this repository), therefore, I have to close this pull request. If you have an idea for a more generic solution fitting this project, I am more than happy to discuss it!

AbdAlftahSalem commented 1 year ago

Thank you for reviewing my pull request and considering my proposal. I completely understand your preference for more common log-level names and your desire for a more generic solution.

After giving it further thought, I believe that the custom log-level feature I suggested can indeed have broader utility for users of this package. While I agree that customizing log levels might not be ideal for the upstream repository, it could be beneficial as an opt-in feature for developers who need more granular logging during development and debugging.

To address your concerns about noise, we can explore implementing this feature more flexibly. One possible solution is to provide an optional custom logger or filter that users can enable if they find it useful. This way, those who need the additional logging granularity can opt in, while others can continue using the default log levels without any change.

Having a success log level can greatly assist developers during testing and debugging when they need to identify specific points of successful execution in their code. It can make it easier to track the flow of asynchronous processes, identify successful API responses, or handle various user interactions that lead to successful outcomes.

I believe that by incorporating this optional feature, we can offer more value to the users of this package without compromising its current functionality or intended design.

Please let me know your thoughts on this approach. If you find it acceptable, I'd be more than happy to work on implementing the custom logger/filter as an alternative solution that aligns with your goals for the repository.

Looking forward to your feedback and further discussion.

Bungeefan commented 1 year ago

Honestly, did you write that with ChatGPT? :wink:

AbdAlftahSalem commented 1 year ago

No not from GPT. Now I can use it :

https://pub.dev/packages/logger_success

It's simple 😄

hpelitebook745G2 commented 1 year ago

hi @Bungeefan , the OP's proposed solution is another package that was forked from your repo. have you considered merging this to your package?

for the record, success log is also a nice-to-have

Bungeefan commented 1 year ago

Hi @hpelitebook745G2, thanks for expressing your opinion on this PR. Unfortunately, the points, stated in my reply to the original issue, are still valid and unsolved, therefore, I am currently not considering merging it.