Formula-Cookbook: Stop recommending rm_f in postinstall
#17965
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- After all the work that went into #17705, we don't want the docs disagreeing with what CI says the style should be.
issyl0
force-pushed
the
formula-cookbook-stop-recommending-rmrf
branch
from
55b7079 to
768ef34
Compare
p-linnane
approved these changes
Aug 4, 2024
MikeMcQuaid
reviewed
Aug 5, 2024
| @@ -32,3 +32,7 @@ Style/RedundantRegexpArgument: | |||
| # Want to be able to display partial formulae in the docs. | |||
| Style/TopLevelMethodDefinition: | |||
| Enabled: false | |||
|
|
|||
| # Formulae and Casks no longer use `rm_f`/`rm_rf`, so the docs need to match. | |||
| Lint/NonAtomicFileOperation: | |||
There was a problem hiding this comment.
Ideally we'd keep this around for mkdir_p.
Is this disabled/enabled in the other Homebrew .rubocop.yml file(s)? If so: should it be adjusted there too?
There was a problem hiding this comment.
It's enabled by default. But it only complained about this one when I removed the f.
There was a problem hiding this comment.
I think that's because this is a non-atomic operation now. Particularly in post-install where we don't fully control the location: this is potentially problematic.
See: https://docs.rubocop.org/rubocop/cops_lint.html#lintnonatomicfileoperation
I think having this still enabled for mkdir_p but also having this be consistent everywhere would be desirable if possible.
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?rm_fandrm_rfin formulae and casks #17705, we don't want the docs disagreeing with what CI says the style should be.