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

Should stop execution if it can't find .inst/apps #235

Merged
merged 4 commits into from
Oct 31, 2023
Merged

Conversation

karangattu
Copy link
Contributor

@karangattu karangattu commented Oct 24, 2023

Fixes #234

The builds are failing though :(

@karangattu karangattu marked this pull request as ready for review October 25, 2023 14:04
@gadenbuie
Copy link
Member

Sorry @karangattu, I just realized that the function signature of fix_snaps() already includes a repo_dir argument.

fix_snaps <- function(
  sha = git_sha(repo_dir),
  ...,
  ask_apps = FALSE,
  ask_branches = TRUE,
  ask_if_not_main = TRUE,
  repo_dir = "."
)

So it might be easier to change the default argument from repo_dir = "." to repo_dir = rprojroot::find_package_root_file().

You'd still keep the check that {repo_dir}/inst/apps exists and error if not, but the error message could point the user to change the repo_dir argument.

Also it'd be a good idea to check that all steps inside fix_snaps() take and respect the repo_dir argument (just by following the code, I mean).

@schloerke schloerke merged commit 63757ec into main Oct 31, 2023
17 of 20 checks passed
@schloerke schloerke deleted the fix/issue-234 branch October 31, 2023 18:47
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.

fix_snaps() should fail if it can't find ./inst/apps
3 participants