-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
RenderRunner cleanup #28496
RenderRunner cleanup #28496
Conversation
R: @robertwb |
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
Codecov Report
@@ Coverage Diff @@
## master #28496 +/- ##
==========================================
+ Coverage 72.22% 72.25% +0.03%
==========================================
Files 684 684
Lines 100856 100854 -2
==========================================
+ Hits 72846 72877 +31
+ Misses 26434 26401 -33
Partials 1576 1576
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Thank you, this all looks good. dotX was leftover from testing, really glad you caught this before the release was cut.
While trying to use the
RenderRunner
, I came across a few warts.run_portable_pipeline
dotX
instead ofdot
for one of its subprocess callsRenderOptions
couldn't be used with.view_as
because the init-time validationRenderRunner
logging used the root logger instead of the module loggerGitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.