-
Notifications
You must be signed in to change notification settings - Fork 155
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
when attempting to load a lockfile and finding only a manifest, automatically generate a lockfile and load that #1980
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.
Overall LGTM; main nits are around style + suggesting we don't need the extra function for finding the path to manifest.json
Co-authored-by: Kevin Ushey <[email protected]>
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!
@kevinushey I forgot to update the |
I'll take care of it, thanks! |
This pull request changes the behavior of
renv_lockfile_load()
, which attempts to load a lockfile from a directory. When no lockfile is found, but amanifest.json
is found, the manifest is used to automatically generate a lockfile, which is then loaded. The behavior only occurs when the function is called withstrict = FALSE
, the default. If no manifest is present, the functionrenv_lockfile_init()
is called, as before. A message is printed when this happens.I added a supporting function in
paths.R
to return a manifest path, just cribbing fromrenv_paths_lockfile()
. However, just doingfile.path(project, "manifest.json")
might be just as good. I'm not sure what the semantics are around path loading, so was just trying to follow existing patterns.The logic in
renv_manifest_path()
is similar torenv_paths_lockfile()
, or at least, the default case. I'm unsure about needing to add an extra layer of abstraction similar torenv_lockfile_path()
->renv_paths_lockfile()
.When testing, I saw a duplicated log line:
This came from explicit logging in
renv_lockfile_from_manifest()
, which duplicated a log message withinrenv_lockfile_write()
, so I removed the duplicated outer logging statement.