-
-
Notifications
You must be signed in to change notification settings - Fork 603
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
feat: expose config resolving as CLI instance #2066
Conversation
0be6565
to
711d394
Compare
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.
Let's migrate on async API everywhere
Codecov Report
@@ Coverage Diff @@
## master #2066 +/- ##
==========================================
- Coverage 68.34% 68.19% -0.15%
==========================================
Files 76 75 -1
Lines 2429 2418 -11
Branches 499 499
==========================================
- Hits 1660 1649 -11
Misses 769 769
Continue to review full report at Codecov.
|
711d394
to
ba67313
Compare
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.
Okay, let's do more improvements in future
@evilebottnawi inlined the function here, need heavy refactor 😞 Will take care |
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.
Looks good, let's refactor it more in other PRs, I have couple ideas
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.
Need rebase after #2092
952fe22
to
aba80e2
Compare
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.
LGTM
f4202d9
aba80e2
to
f4202d9
Compare
very strange tests 😞 |
What kind of change does this PR introduce?
feat
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
NA
Summary
Expose config resolving on CLI instance for architecture
Does this PR introduce a breaking change?
No
Other information
/cc @evilebottnawi