r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.19k stars 184 forks source link

Use the latest major `{withr}` version #2583

Closed IndrajeetPatil closed 4 months ago

IndrajeetPatil commented 4 months ago

To benefit from performance and other enhancements in this release: https://www.tidyverse.org/blog/2024/01/withr-3-0-0/

We have already fixed issues stemming from this version: https://github.com/r-lib/lintr/pull/2519

We didn't bump the version because it was quite new at the time, but we can do so now.

MichaelChirico commented 4 months ago

Sorry, I don't see why we should increase the minimum version dependency unless we actually use something from that version?

If some user can't upgrade {withr}, why should we prevent them from using {lintr} unless there's something specific in the new version that we "need" for {lintr}?

I must be missing something...

IndrajeetPatil commented 4 months ago

If some user can't upgrade {withr}, why should we prevent them from using {lintr}

But it's not a runtime dependency of {lintr}. Why would it prevent them from using {lintr}?

IndrajeetPatil commented 4 months ago

At any rate, this is not critical. withr 3.0.0 was quite an extensive rewrite, and I would have liked for us to see some of its benefits in our dev workflow (performance, better messages, etc.).

Maybe working on https://github.com/r-lib/lintr/issues/2539 will help motivate this further.

MichaelChirico commented 4 months ago

But it's not a runtime dependency of {lintr}. Why would it prevent them from using {lintr}?

True, but all the more reason not to enforce a minimum version IMO.

If there's some actual code changes you want to make that use updated functionality, I am happy to proceed.