-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix circular import #11137
Fix circular import #11137
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11137 +/- ##
==========================================
- Coverage 88.91% 88.86% -0.06%
==========================================
Files 183 186 +3
Lines 23937 23971 +34
==========================================
+ Hits 21284 21302 +18
- Misses 2653 2669 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
|
wow, I didn't know I fixed an issue! good job me! |
The one check CI is uhappy with is code coverage, here: https://github.com/dbt-labs/dbt-core/pull/11137/checks?check_run_id=34330125418
So probably what's happened here is that we've added 1 line of imports (which is not considered covered even if it is actually exercised by tests), and then removed ~10 lines of bugs & duplication. These removed lines were covered, and thus coverage has gone down (it's also possible that we're accidentally counting changelog lines as uncovered code, but this is less likely). Anyway, the moral of the story is that code coverage is not an ultra-precise measure of code quality. We could add a few lines of no-ops to an otherwise-covered function to "fix" this, but I don't recommend this. |
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.
This looks good to me, thanks for adding this @dradetsky !!
resolves #11142
Problem
There was a circular import which required duplicating a method in
core/dbt/config/profile.py
. This was both ugly and an impediment to improving that method if required (as in #11108).Solution
Fix the circular import and remove the duplication
Checklist