Fix redundant `load_all()` call in `devtools::test()`

r-lib / devtools

Tools to make an R developer's life easier

https://devtools.r-lib.org

Other

2.37k stars 753 forks source link

Fix redundant `load_all()` call in `devtools::test()` #2436

Closed lionel- closed 2 years ago

lionel- commented 2 years ago

Currently I'm seeing:

> devtools::test('~/Sync/Projects/R/r-lib/rlang')
ℹ Loading rlang.
ℹ Testing rlang
ℹ Loading rlang.

With this PR this becomes:

> devtools::test('~/Sync/Projects/R/r-lib/rlang')
ℹ Testing rlang.
ℹ Loading rlang.

hadley commented 2 years ago

FWIW you're seeing this now because of a pkgload bug: https://github.com/r-lib/pkgload/issues/213. I guess it's good that your pkgload bug revealed my devtools bug 😆

lionel- commented 2 years ago

Thanks for discovering my bug @hadley :)

Regarding the second commit which adds a trailing dot, I now have a doubt. In error messages we recommend ending messages with full stops. But what is our standard/recommendation for other UI alerts? Should they be consistent?

hadley commented 2 years ago

It looks weird with a trailing period to me, but that might just be what I'm accustomed too. OTOH these are mostly sentence fragments, not full sentences so maybe . isn't needed?

lionel- commented 2 years ago

ok I've removed the trailing dot here, and in pkgload: https://github.com/r-lib/pkgload/commit/78d5cb174c5cb7c298265ea953a038b57e640911