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

Fixes `:all` Logger metadata handling from MissedMetadataKeyInLoggerConfig check #1035

Closed antedeguemon closed 11 months ago

antedeguemon commented 1 year ago

Hey folks! 🖖

Credo 1.7 was released with the MissedMetadataKeyInLoggerConfig check. The check ensures all Logger calls have their metadata fields defined in the configs.exs - in other words, it ensures all metadata fields are really logged and no metadata field is ignored from the logs.

There is a small issue with the new check:

The issue is that when the value is :all, the check crashes. 😿

What this PR does?

Even though it doesn't make much sense to run MissedMetadataKeyInLoggerConfig check for the codebases where all metadata fields are already logged, the check is enabled by default Credo configs.

So this pull request fixes the problem by skipping issues when metadata_keys == :all.

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.

cpiemontese commented 1 year ago

This should close #1033

rrrene commented 1 year ago

This is a great find (we obviously missed :all).

But why would we look at every single potential issue to skip issue creation if :all is set.

Couldn't we skip the check altogether in that case?

rrrene commented 1 year ago

@antedeguemon ping

antedeguemon commented 11 months ago

Hey folks!

Sorry about the year-long delay on this pull request, life got in the middle.

I'm closing this since https://github.com/rrrene/credo/pull/1044 already fixed it. Kudos to @davorbadrov.

rrrene commented 11 months ago

@antedeguemon No worries. We all have lifes outside open source, even outside the internet. ✌️

Thank you to everyone contributing to projects in their spare time. It is a real net positive ❤️