Skip to content
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

orderly_artefact() requires description vs. orderly_resource() only file list #150

Open
jeffeaton opened this issue Jul 4, 2024 · 3 comments

Comments

@jeffeaton
Copy link

On using orderly2 with R function declarations, the arguments feel somewhat incongruous:

  • orderly_artefact() requires a description = argument
  • orderly_resource() only accepts a list of files (no description attached)

Is there a conceptual reason why an artefact needs a description (rather than only requiring declaration of the files)?

Intuitive harmonised interface for me:

  • orderly_artefact() requires files = argument, description = argument becomes an optional argument (ideally second)
  • orderly_resource() gains an optional description = argument
  • orderly_dependency() gains an optional description = argument

Maybe there's a reason that doesn't make sense

@richfitz
Copy link
Member

richfitz commented Jul 4, 2024

No, this makes sense, and should only be minimally disruptive if we make it right now. The reason for the existing behaviour was it mirrored the previous yaml most closely

@richfitz
Copy link
Member

richfitz commented Jul 5, 2024

So, looking further - it will be easy enough to make the description of artefact optional, though this will require a period of adjustment before we can change the argument order, based on patterns of usage.

It's a bit harder to get descriptions added to resource and dependency though, because of the way these are stored in the final metadata. We can change this, but it's a more disruptive change than I would like to do lightly, even if things are still new, partly because nobody previously asked for this in orderly1 (so it's not clear that it would warrant the disruption). I'll get onto making this optional for artefacts though...

@plietar
Copy link
Member

plietar commented Jul 8, 2024

While we are at it, it might be worth changing orderly_shared_resource to take a list vector of files as well, instead of a .... This had come up when I was doing #136.

richfitz added a commit that referenced this issue Jul 15, 2024
Harmonise orderly_artefact() with orderly_dependency etc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants