Open Ldoppea opened 2 years ago
Moreover I think the log method has 4 required parameters, but we notice 2 to 3 parameters filled for most of its usage. I suggest to make explicit the required parameters, document their use with JSDoc, and add one example in the README
And I don't see the minilog website working but it got mention in our documentation: https://docs.cozy.io/en/cozy-client/logging/ .
Current cozy-logger implementation may be troublesome for future maintainers. Here are some questions/remarks I had in my head when reading the code:
filterOut()
method's name is suggesting that it's filtering something, but it just returns a boolean. Then the caller decides to filter it or not regarding the boolean.filterOut()
implementation is suggesting that all filters should return a boolean, butfilterSecrets()
can only returnvoid
or throw an exception.secret
log level for? It is not documented, and filters prevent it to be called. Is it used somewhere?filterOut
filterOut
returnstrue
: object should be filteredfilterLevel
) returnstrue
: object should NOT be filteredfilterLevel()
always return false if the given type does not exist in thelevels
array. This is intended but I think we should make this behavior more visible (even if this creates redundant code)setNoRetry()
seems not to be used (except from unit tests). Is it dead code?What do you think about this?
I would suggest to refactor this code, but as it is used in a lot of places and as it is stable code (not edited during last 3 years) this choice may be discutable.