-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Update to update_*_defaults()
#5781
Conversation
TODO: look into feasibility of the suggestion here: #4993 (comment) |
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.
apart from comment lgtm
R/geom-defaults.R
Outdated
if (!index %in% ls(cache_defaults)) { | ||
cache_defaults[[index]] <- old | ||
} |
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.
Maybe overengineering it, but this is a rather unperformant way to check for existence in an environment as it ignores the whole hash structure. exists()
are there to check for bindings in an environment
This reverts commit 60407ac.
This reverts commit 60407ac.
This reverts commit 60407ac.
* unify updating mechanism * add tests * redocument * add news bullet * add review suggestion
This PR aims to fix #4993 and supersedes #5098.
Briefly:
new = NULL
.In contrast to #5098: