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

Add JSON lockfile schema and validation function #1889

Merged
merged 3 commits into from
May 18, 2024

Conversation

jrdnbradford
Copy link
Contributor

Here's my first go at issue #1877.

Why Use JSON Schema:

  • Provides an easy, declarative way to document renv.lock
  • Integrates with text editors to provide autocompletion, hover help, and validation
  • Allows automated validation in CI, builds, tests, etc for R applications

General Decisions

  • A relatively relaxed schema to prevent false negatives, with the option for users to pass their own schema that fits their personal requirements. Additional fields are allowed.
  • Heavy on positive and negative testthat tests, so that we can verify updates to the schema
  • The function wraps jsonvalidate::json_validate() and takes/passes many of its parameters there. I have not implemented all of them, but we can add if you’d like.
  • Ease of use. Calling without parameters should cover most cases: renv::lockfile_validate()

Questions

  • Do I have all the fields renv knows about?
  • Do I have required set only for the fields that are absolutely necessary?
  • I’ve used descriptions from renv documentation whenever possible, but had to make some inferences. I myself have never used anything that added Bioconductor, Python, or a few other fields. These should be checked.

Copy link
Collaborator

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM -- thank you!

R/lockfile-validate.R Outdated Show resolved Hide resolved
R/lockfile-validate.R Show resolved Hide resolved
jrdnbradford and others added 2 commits May 18, 2024 10:50
@jrdnbradford
Copy link
Contributor Author

Updates have been pushed.

Presuming this gets merged, feel free to @ me on issues, bugs, etc on this. Schemas and their validation is always a balancing act, and the R community at large may need different tradeoffs.

Closes #1877.

@kevinushey kevinushey merged commit 568e8cc into rstudio:main May 18, 2024
@kevinushey
Copy link
Collaborator

A huge thanks again for putting this together!

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

Successfully merging this pull request may close these issues.

None yet

2 participants