search

openpharma / staged.dependencies

R package to implement development stages for package development
https://openpharma.github.io/staged.dependencies/
MIT License
12 stars 3 forks source link

Don't override user-set option for storage directory #171

Closed asbates closed 1 year ago

asbates commented 1 year ago

This PR fixes an issue where if a user had set the staged.dependencies._storage_dir, it was being overwritten in setup_storage_dir. It also adds a new function set_storage_dir to allow changing the storage directory after package load.

Additionally, copy_config_to_storage_dir was modified to not fail if the config file already exists. It now only tries to copy the file if it does not exist, and fails only if the copy failed.

nikolas-burkoff commented 1 year ago

@cicdguy -> I assume R CMD CHECK failing due to problems with the action, it's asking for desc package which is definitely in the lockfile...

cicdguy commented 1 year ago

@cicdguy -> I assume R CMD CHECK failing due to problems with the action, it's asking for desc package which is definitely in the lockfile...

I could switch the workflows for roxygen and R CMD check to use Admiral's workflows, since they have proper renv.lock support. The container-based workflows clearly aren't working super great here.

nikolas-burkoff commented 1 year ago

I could switch the workflows for roxygen and R CMD check to use Admiral's workflows, since they have proper renv.lock support. The container-based workflows clearly aren't working super great here.

Yup that makes sense

asbates commented 1 year ago

@nikolas-burkoff Please DO NOT MERGE. I'm having some issues so I think I've missed something.

asbates commented 1 year ago

@nikolas-burkoff Please DO NOT MERGE. I'm having some issues so I think I've missed something.

Crisis averted.

nikolas-burkoff commented 1 year ago

@asbates is this ok to merge now

@cicdguy is the CI/CD happy yet?

asbates commented 1 year ago

@asbates is this ok to merge now

@cicdguy is the CI/CD happy yet?

Yup, all good from my end.

cicdguy commented 1 year ago

I'd say YOLO and merge it. I'll deal with the checks etc later. Don't want to hold this up