extend/pathname: import rmtree again. #17329
Conversation
| # Ideally we'd odeprecate this but probably can't given gems so let's | ||
| # create a RuboCop autocorrect instead soon. | ||
| # This is why monkeypatching is non-ideal (but right solution to get | ||
| # Ruby 3.3 over the line). | ||
| # odeprecated "rmtree", "FileUtils#rm_r" |
There was a problem hiding this comment.
Just a comment re the autocorrect: in my experience, use of #rmtree in formulae typically does mean rm_rf, so I wonder if we could avoid the unnecessary code churn, at least for formulae.
There was a problem hiding this comment.
We are using both rm_rf and rmtree in formulae currently. Might make sense to only use rm_rf anyways for simplicity and explicitness.
There was a problem hiding this comment.
I'd discourage rm_rf unless you are ok with it not actually removing the directory. rm_r is safer where possible
There was a problem hiding this comment.
in my experience, use of
#rmtreein formulae typically does meanrm_rf
Not sure what you mean about this. Do we have code that makes Pathname operations ignore all errors?
There was a problem hiding this comment.
Oh right, I think you are talking about rmtree something rather than something.rmtree. Yes they mean two different things with the former meaning rm_rf and the latter meaning rm_r. Encouraging rm_r something everywhere could make sense.
There was a problem hiding this comment.
@carlocab I think there's an argument that this could be the beginning of "monkey patch Pathname everywhere" (as we do today) to "only monkey patch Pathname and similar in formulae/casks and otherwise use things as-is".
I would really like to see us get rid of pretty much all extends of core/stdlib Ruby classes.
The separate file is no longer required now that sorbet/sorbet#7895 was merged.
The separate file is no longer required now that sorbet/sorbet#7895 was merged.
(if this fails, it'll be that we need a Sorbet version bump first)