-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
evaluate call in base env (#861) #863
Conversation
Codecov Report
@@ Coverage Diff @@
## master #863 +/- ##
======================================
Coverage 90.3% 90.3%
======================================
Files 70 70
Lines 3261 3261
======================================
Hits 2945 2945
Misses 316 316 Continue to review full report at Codecov.
|
Thanks! It should also have no adverse effects on tracebacks, unlike the inlining solution. |
Thanks. You're fire today :) Thanks for all that. I guess I need to figure what I'll turn on to rev.dep. I guess I should start with Lionel's PR with the extra flag set. |
Passes fine in full rev.dep (one level) so merging (after adjusing by hand here for merge conflicts). |
@eddelbuettel There was an issue with the merge conflict resolution, this PR only contains Changelog changes. |
Boo. Well, master is master. Either we all manage to produce clean PRs that actually change code, or we just paint the bike shed. Looks like this time we focused on the bikeshed. |
@lionel- @kevinushey @thirdwing : As Lionel pointed out here, something went amiss with the PRs. I only ever resolved conflicts (on the website) pertaining to |
I should be able to fix things up on master. |
Maybe I was blind (it was late when I came across @lionel- 's message) but I didn't even see the correct fragment in master. I guess I may been dense and been on another branch... Thanks for the fix, @kevinushey. |
This is the simpler version of my prior patch for #861 (as recommended by @lionel-). If this builds cleanly I think it should be good to merge