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

Misc cleanup #52

Merged
merged 13 commits into from
Feb 1, 2024
Merged

Misc cleanup #52

merged 13 commits into from
Feb 1, 2024

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Jan 31, 2024

fixes #45
You do not need to have that many workflows. To add gh actions to your package can be done by calling usethis::use_github_action().

You don't need load_all() in README.Rmd. You can run devtools::build_readme() to re-render README.

You also do not need

For the single vignette that requires load_all(), I added pkgload to Suggests. (devtools::load_all() only re-exports pkgload::load_all().

On top of that, I included minor tests lints. and namespacing in vignettes.

Cheers!

@lhdjung
Copy link
Owner

lhdjung commented Feb 1, 2024

Thanks so much, this is very helpful! I will check it out in more detail later.

@olivroy
Copy link
Contributor Author

olivroy commented Feb 1, 2024

If possible, could you approve the CI run, so that I can fix any problems I didn't encounter locally ahead of your review?

I also tried to separate as much as possible each step in a commit with a descriptive name.

Edit:

the check passes.

However, there is a Note

* checking R code for possible problems ... NOTE
Error: Error in attr(e, "srcref")[[i]] : subscript out of bounds
Calls: <Anonymous> ... <Anonymous> -> collectUsage -> collectUsageFun -> walkCode -> h
Execution halted
debit_map_seq: Error while checking: subscript out of bounds
debit_map_total_n: Error while checking: subscript out of bounds
grim_map_seq: Error while checking: subscript out of bounds
grim_map_total_n: Error while checking: subscript out of bounds
grimmer_map_seq: Error while checking: subscript out of bounds
grimmer_map_total_n: Error while checking: subscript out of bounds

If you have an idea of where this comes from and how to fix, I will be happy to check it out!

@lhdjung lhdjung merged commit 0239cc6 into lhdjung:devel Feb 1, 2024
5 checks passed
@lhdjung
Copy link
Owner

lhdjung commented Feb 1, 2024

Merged the PR. Thank you again for your detailed work, @olivroy!

The workflow setup used to be a bit messy because I mixed several usethis functions for Github Actions, then scrambled to fix build failures. Your new setup looks much cleaner.

I didn't get this note on my machine, but it seems to be related to one of the function factories. (All functions listed in the note were created by either function_map_seq() or function_map_total_n().) This is a very complicated part of the package, and I will check this out manually.

@olivroy
Copy link
Contributor Author

olivroy commented Feb 1, 2024

Of course, feel free to ping me if I broke anything or if you want something reverted. I don't want to do more harm than good!
It is possible that it is in vignettes where I removed the load_all() usage, because it didn't show problems for me locally.

Also, I saw a .Rprofile and a .Rproj file under the R/ directory. You likely do not need those as all the work can be conducted from the top-level directory with devtools.

You can always get the latest version of an action by calling usethis::use_github_action(). It will overwrite the one you currently have.

@olivroy olivroy deleted the fixes branch February 1, 2024 20:45
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.

2 participants