-
Notifications
You must be signed in to change notification settings - Fork 72
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
Convert get_config()
to direct imports of CONFIG
#2622
Conversation
Passing run #335 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
Codecov ReportBase: 86.47% // Head: 86.46% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #2622 +/- ##
==========================================
- Coverage 86.47% 86.46% -0.02%
==========================================
Files 289 289
Lines 16009 15955 -54
Branches 2019 2020 +1
==========================================
- Hits 13844 13795 -49
+ Misses 1782 1778 -4
+ Partials 383 382 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@sanders41 do you know how to get code cov to ignore print statements as part of coverage? Or maybe what I can do to test it to make code cov happy? |
You can test prints with something like: def my_print():
print("My print string")
def test_my_print(capfd):
my_print()
out, _ = capfd.readouterr()
assert "My print" in out It is also possible to ignore in coverage. I try to save this for places where I here tried everything and can't get the code in a state to hit the line in tests. def my_print():
print("My print string") # pragma: no cover |
I have some print statement tests elsewhere but here I don't feel it is really necessary.... Will ignore it for now, thanks! |
src/fides/api/ctl/migrations/versions/d65bbc647083_adds_gpc_info_to_consent_table.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of nits, but a large and useful change, thanks @ThomasLaPiana
Closes #2611
Code Changes
get_config
with a directCONFIG
import (with a few exceptions where needed)CONFIG = get_config()
(with a few exceptions where needed)verbose
parameter inget_config
Steps to Confirm
Loading config...
(it only loads verbosely when called via the CLI, not on application load)Pre-Merge Checklist
Documentation:Relevant Follow-Up Issues CreatedCHANGELOG.md
Description Of Changes
There are a huge number of file changes here but the change itself is relatively minor, and all changes are described in the
Code Changes
section. The automated tests are the main way we'll know if this is functioning as expected or not.NOTE: There is a code cov failure but I'm not sure why or how to fix it, so I'm asking the reviewer to approve in spite of this and I'll follow up separately