You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This might be a kind of minor issue, but I think it would be both useful and good practice to add an option to disable the logged messages. In my opinion there really shouldn't be a bunch of non-configurable print statements in a package like this. However it's obviously very useful to have these messages for debugging purposes. Therefore I propose to, by default, only print them when the app is in debug mode but to provide an option to also force them to be disabled/enabled.
The text was updated successfully, but these errors were encountered:
That is a good suggestion. Instead of having it only print in debug mode, maybe we can pass a configuration parameter that controls the print operation. Because I can think of use cases where someone would want to see the logs in release build also.
Yes for sure. Thats what I proposed too actually.
My initial idea would simply be an optional boolean named something like logDebug. If it is null we only log if we are in debug mode and if it isn't null, we log if it is true. This would be a breaking change though, as we would disable logging in release builds for people who haven't added the configuration parameter yet.
An alternative would be passing an enum like the following:
That way, the default value could be SSELoggingConfig.always, therefore keeping the current behaviour while providing convenient options too.
I don't know if this "breaking change" introduced by the first option would be critical enough to instead go with the admittedly kinda weird enum solution. I think it's questionable wether some people are currently relying on these debug messages in release builds.
Of course the simplest option of all would be to leave out the debugMode thing altogether and just add a boolean option defaulting to true.
This might be a kind of minor issue, but I think it would be both useful and good practice to add an option to disable the logged messages. In my opinion there really shouldn't be a bunch of non-configurable print statements in a package like this. However it's obviously very useful to have these messages for debugging purposes. Therefore I propose to, by default, only print them when the app is in debug mode but to provide an option to also force them to be disabled/enabled.
The text was updated successfully, but these errors were encountered: