Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check for piping into Logger functions #1182

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hkrutzer
Copy link

@hkrutzer hkrutzer commented Mar 2, 2025

Calls into Logger can be purged when :compile_time_purge_matching is enabled in the Logger configuration causing unexpected behavior in releases.

See https://x.com/elixirboy/status/1770336803280556486 and https://bsky.app/profile/hauleth.dev/post/3lhguoqmpos2i for examples of this happening to people.

@@ -0,0 +1,55 @@
defmodule Credo.Check.Warning.PipeToLogger do

Check warning

Code scanning / Credo

Modules should have a @moduledoc tag.

Modules should have a @moduledoc tag.
@rrrene
Copy link
Owner

rrrene commented Mar 3, 2025

Hi, thx for putting this together.

But please don't link several social media posts on different networks (which not everybody might have an account for) instead of writing a simple rationale. 🙂

It is also bad for reviewers to come back to something like this and then having to open multiple tabs again, just to re-familiarize themselves. 👍

@hkrutzer
Copy link
Author

hkrutzer commented Mar 3, 2025

Thanks @rrrene the description was not intended to be a rationale, but to argue the merit of the check. The rationale is in the description of the check. I understand if that wasn't clear. I added part of the check description to the PR description 👍

@hkrutzer hkrutzer force-pushed the logger_pipeline_check branch from 7932035 to 2f5038c Compare March 3, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants