-
Notifications
You must be signed in to change notification settings - Fork 2
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
Harmonise orderly_artefact() with orderly_dependency etc #160
Conversation
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.
Is there any part of Packit that uses the artefacts from the metadata to view packets?
R/metadata.R
Outdated
assert_scalar_character(description, call = environment()) | ||
orderly_artefact <- function(description = NULL, files) { | ||
if (!is.null(description)) { | ||
assert_scalar_character(description, call = environment()) |
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.
How about assert_scalar_character(description, call = environment(), allow_null = TRUE)
?
Quick grep through gives me a handful of occurrences of this pattern.
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.
thanks, good idea
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!
This PR does part 1 of harmonising
orderly_artefact()
withorderly_resource
and friends:description
argument, which other functions lack becomes optional in the schema and the function definitionThis latter change, once in the wild, will make it less disruptive to change the order of the arguments, so that we have
See #150 for context