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

Embrace {usethis} #299

Open
Ilia-Kosenkov opened this issue Jun 28, 2023 · 7 comments
Open

Embrace {usethis} #299

Ilia-Kosenkov opened this issue Jun 28, 2023 · 7 comments
Labels
question Question/discussion

Comments

@Ilia-Kosenkov
Copy link
Member

Recent PR #292 shows that we heavily rely on {usethis} and only pretend we can work without it. At this point, I see no particular reason to not depend on {usethis} and drop all hacks that we have in place.

Pinging
@CGMossa @multimeric @yutannihilation @eitsupi @JosiahParry

@Ilia-Kosenkov Ilia-Kosenkov added the question Question/discussion label Jun 28, 2023
@Ilia-Kosenkov Ilia-Kosenkov added this to the vNext milestone Jun 28, 2023
@eitsupi
Copy link
Contributor

eitsupi commented Jun 28, 2023

When using functions to create packages, it makes sense to assume that usethis is installed (The rextendr::document function will not work without more massive devtools than usethis!).

However, since this package also has functions unrelated to package creation, it would make sense to keep usethis as an optional dependency.

@JosiahParry
Copy link
Contributor

This topic has been raised again today. I think my personal preference on how to handle this would be to completely error out for functions that require the use of usethis rather than still succeeding but with work arounds that omit the use of usethis.

I think this can be accomplished with rlang::check_installed("usethis") which provides the user with an interactive prompt. If they choose to install the package the function continues and if not, the function errors out.

@CGMossa
Copy link
Member

CGMossa commented Nov 17, 2023

Alright, I personally would go with @JosiahParry solution. If you're up for such a PR, or @kbvernon for that matter, I think that's a good way forward.
These packages won't be maintained by us forever, and they need to be in slick shape for the future (mostly also for the god damn present).

@kbvernon
Copy link
Contributor

I can cast an eye over your R code to check for usethis calls and add rlang::check_installed("usethis") + if (!requireNamespace("usethis")) guards as Josiah suggests. The "workarounds" that come after the else might be a little much for me though, but I can try.

@Ilia-Kosenkov Ilia-Kosenkov removed this from the vNext milestone Jan 31, 2024
@Neutron3529
Copy link

I'm creating a package, which could be directly build with R CMD check.
Several changes:

  1. Change lib type from ["staticlib"] to ["cdylib"], which will let cargo generate a .so/.dll file directly.
  2. Edit the Makevars, edit $(STATICLIB) target to STATLIB = $(LIBDIR)/librext.so (since I have no knowledge how to tell Makevars to build SHLIB directly.)
  3. For target $(STATICLIB), add a cp $(STATLIB) . instruction since there is no need link *.a to *.so anymore.
  4. Write useDynLib(libname,...) rather than useDynLib(name,...) in NAMESPACE.

I'm currently working with such things, wondering whether any of you need help.

@Ilia-Kosenkov
Copy link
Member Author

@yutannihilation ,hi, I know you did a lot of research on the topic, any thoughts?

@CGMossa
Copy link
Member

CGMossa commented Jun 11, 2024

@Neutron3529 Hallo! I think it is nice that you're experimenting with these. We've done a ton of these experiments, and happy to follow along with similar journeys. I would like this to be in its own issue rather than be jammed into this issue about {usethis}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Question/discussion
Projects
None yet
Development

No branches or pull requests

6 participants