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

change verbosity level to normal #13

Merged
merged 7 commits into from
Dec 8, 2023
Merged

Conversation

deronnax
Copy link
Contributor

@deronnax deronnax commented Dec 2, 2023

So I had the confirmation in pdm#2460 that the verbosity level is actually normal, so it's up to pdm-dotenv to change its verbosity level. With this change, the message still shows up on normal invocation and doesn't when --quiet is passed to pdm:

➜  pdm venv activate
Loading dotenv file /Users/mathieu/dev/pdm-dotenv/.env
source '/Users/mathieu/Library/Application Support/pdm/venvs/pdm-dotenv-LmuwbYeK-3.10/bin/activate'
➜  pdm venv activate --quiet
source '/Users/mathieu/Library/Application Support/pdm/venvs/pdm-dotenv-LmuwbYeK-3.10/bin/activate'

Copy link

codecov bot commented Dec 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6859926) 100.00% compared to head (357cdf9) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #13   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           12        13    +1     
=========================================
+ Hits            12        13    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@znd4
Copy link
Owner

znd4 commented Dec 3, 2023

This looks great; thanks for taking the time to track this down :)

I'm gonna spend some time today trying to add some tests for this, including a bit a of refactoring, but if I haven't merged this PR by later in the week, feel free to nudge me here and I'll merge and release without them b.c. the downside risk of this change seems pretty small

@deronnax
Copy link
Contributor Author

deronnax commented Dec 3, 2023

Cool. Thank you.

@deronnax
Copy link
Contributor Author

deronnax commented Dec 4, 2023

Hey, it's actually causing some problems for us, it would be super nice if our devs could already use it. Would you please merge it and release, even it not covered by tests?

Copy link
Owner

@znd4 znd4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, just saw your latest comment earlier tonight!

--quiet doesn't seem to work right after pdm (at least as I'd expect), although it works after the subcommand.

That being said, I just saw that there's an option to output to stderr instead of stdout, which should hopefully fix pdm venv activate. In case it doesn't, pdm venv --quiet activate should work now

@znd4 znd4 merged commit 9017a00 into znd4:main Dec 8, 2023
20 checks passed
@deronnax
Copy link
Contributor Author

deronnax commented Dec 8, 2023

I take note. Thank you very much for the merge 🙇

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