rrrene / credo

A static code analysis tool for the Elixir language with a focus on code consistency and teaching.
http://credo-ci.org/
MIT License
4.93k stars 417 forks source link

fix logger metadata check when metadata :all set #1044

Closed davorbadrov closed 1 year ago

davorbadrov commented 1 year ago

Hello, this is another take on this PR https://github.com/rrrene/credo/pull/1035 which fixes the metadata: :all behavior for MissedMetadataKeyInLoggerConfig check.

The issue with the check is that it doesn't properly support the metadata: :all configuration. This PR is different from the linked one because it skips running the check entirely if metadata: :all is set (since it makes no sense to run it), I've also added a new test that covers the scenario.

Please let me know if it needs to be tweaked before merging.

What this PR does?

This pull request prevents credo from crashing when metadata_keys == :all and when MissedMetadataKeyInLoggerConfig check runs.

More details

From Logger.Backends.Console documentation:

:metadata - the metadata to be printed by $metadata. Defaults to an empty list (no metadata). Setting :metadata to :all prints all metadata. See the "Metadata" section for more information.

Nezteb commented 1 year ago

For what it's worth I pulled this locally, built credo, tested with it, and it works with the metadata: :all logger config. ๐Ÿ˜„

Nezteb commented 1 year ago

Also just to link to the original issue: https://github.com/rrrene/credo/issues/1033

cc @rrrene ๐Ÿ˜„

rrrene commented 1 year ago

@davorbadrov @Nezteb Thx! ๐Ÿงก