search

gesistsa / rio

🐟 A Swiss-Army Knife for Data I/O
http://gesistsa.github.io/rio/
600 stars 76 forks source link

`.check_trust()`: Is there a way to NOT encouraging users to report the issue here? #451

Closed chainsawriot closed 2 weeks ago

chainsawriot commented 1 month ago

https://github.com/gesistsa/rio/blob/447ea1b9b2f50761f8a646d4adb95e2a270175c0/R/utils.R#L145-L150

chainsawriot commented 1 month ago

r-lib/lifecycle#187

As well as #450 #432

chainsawriot commented 1 month ago

452 I think we need to address this before the typical "semester beginning" wave.

chainsawriot commented 1 month ago

tag @schochastics @jsonbecker

I think one solution is to not use lifecycle for this. Just use the good old warning().

jsonbecker commented 1 month ago

Oh wow that's aggressive to report an issue to a maintainer when they've deprecated a feature?

I'm good with swapping this to warning but that's a bad upstream choice.

schochastics commented 1 month ago

That is indeed a bit aggressive. I second that a good old warning is enough.

chainsawriot commented 2 weeks ago

Maybe should give this suggestion by @olivroy a try: r-lib/lifecycle#187

chainsawriot commented 2 weeks ago

But how deep in the caller_env() should we go? It is still quite convoluted.

schochastics commented 2 weeks ago

I think I still vote for an ordinary warning but mostly because I have not yet fully understood the caller_env()

chainsawriot commented 2 weeks ago

@schochastics OK, I agree with you. And I close this once again.

olivroy commented 2 weeks ago

Agreed the warning seems fine.

Here is my attempt at explaining caller_env()

https://github.com/wjakethompson/taylor/pull/49#issuecomment-2197134362

This guide can be helpful too.

https://rlang.r-lib.org/reference/topic-error-call.html

I made a few PRs to implement this in different packages.

See lintr for example https://github.com/r-lib/lintr/pull/2602 or gt https://github.com/rstudio/gt/pull/1638