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

onload_package_startup_linter for packageStartupMessage() inside .onLoad()? #882

Closed
MichaelChirico opened this issue Nov 8, 2021 · 4 comments · Fixed by #883
Closed

Comments

@MichaelChirico
Copy link
Collaborator

Using packageStartupMessage() in the .onLoad() hook is an R CMD check NOTE. So I think it makes sense to offer a linter to detect that as an early warning.

@AshesITR
Copy link
Collaborator

AshesITR commented Nov 8, 2021

This seems useful but oddly specific.
Are there similar detectable problems? e.g. using message() in .onLoad or .onAttach?

@MichaelChirico
Copy link
Collaborator Author

I'll experiment later, but from my read it really only is packageStartupMessage() in .onLoad():

https://github.com/wch/r-source/blob/8b6625e39cd62424dc23399dade37f20fa8afa91/src/library/tools/R/QC.R#L5167

The line before suggests we could include library.dynam() calls in .onAttach() as well... we could add that & rename it as ns_hook_usage_linter? WDYT?

@AshesITR
Copy link
Collaborator

AshesITR commented Nov 9, 2021

What about package_startup_linter as a name and we also check, e.g.

https://github.com/wch/r-source/blob/8b6625e39cd62424dc23399dade37f20fa8afa91/src/library/tools/R/QC.R#L5254-L5267

@MichaelChirico
Copy link
Collaborator Author

Nice! Good idea

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 a pull request may close this issue.

2 participants