Allow a root for deps-in-desc #432
Conversation
lorenzwalthert
force-pushed
the
issue-429
branch
from
1ece710 to
47b6df4
Compare
| @@ -16,6 +18,8 @@ pre_installed <- c( | |||
| ) | |||
|
|
|||
| arguments <- docopt::docopt(doc) | |||
| arguments$files <- normalizePath(arguments$files) # because working directory changes to root | |||
There was a problem hiding this comment.
I don't know if we want to normalize these paths. Doesn't that remove the purpose of this --root option?
If we normalize files before setwd(normalizePath(arguments$root)), then this hook called like below:
deps-in-desc --root "rpkg" FILE1
will process rpkg/../FILE1
However, I think we would want it to process rpkg/FILE1
Therefore, this would require:
deps-in-desc --root "rpkg" rpkg/FILE1
Am I missing something?
There was a problem hiding this comment.
I am not sure I can follow, but pre-commit will generate a call where the paths to the files are relative to the git root. Like this
Rscript inst/hooks/exported/deps-in-desc.R --root rpkg rpkg/file1
So we need to make these paths absolute before changing the working directory, otherwise information is lost. Afterwards, it does not matter anymore if the paths are absoulte or if we convert back to relative. The working directory change is only for ensuring desc::desc() finds the DESCRIPTION file. And it could also be solved without changing the working directory in the first place but I think the current approach genearlised better to other situations.
There was a problem hiding this comment.
I just pushed a fix for tests because previously, the R source file to test was not in the R sub directory, but one level above. This also works, i.e. the R source file can be outside of the R package root to test if the dependencies are captured, but probably that's not what we want to test.
There was a problem hiding this comment.
I am not sure I can follow, but pre-commit will generate a call where the paths to the files are relative to the git root. Like this
Rscript inst/hooks/exported/deps-in-desc.R --root rpkg rpkg/file1So we need to make these paths absolute before changing the working directory, otherwise information is lost. Afterwards, it does not matter anymore if the paths are absoulte or if we convert back to relative. The working directory change is only for ensuring
desc::desc()finds the DESCRIPTION file. And it could also be solved without changing the working directory in the first place but I think the current approach genearlised better to other situations.
Ah, I thought the files were being passed in by the user and not from precommit itself. I now understand the need to normalize the files.
…s, not one level above
this now has new system dep: r-lib/devtools@575ae4e
|
Now, there are remaining hooks to adapt:
|
|
@TNonet are you interested in contributing? |
|
Hi @lorenzwalthert, Yes, thank you for the patience/support. I worked on this (#436) morning. I am unsure what is wrong with my new |
|
Ok, great. I go ahead and merge this. Please rebase your branch on this if possible. |
No description provided.