-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
utils: reduce odeprecated warnings. #3725
utils: reduce odeprecated warnings. #3725
Conversation
# Don't throw deprecations at all for cached or .brew formulae. | ||
return if backtrace.any? do |line| | ||
line.include?(HOMEBREW_CACHE) || line.include?("/.brew/") | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative that I find cleaner but don't feel too strongly on:
return if backtrace.grep(/#{Regexp.escape(HOMEBREW_CACHE)}/)
return if backtrace.grep(%r{/\.brew/})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alyssais Good call, changed (to use a string version)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alyssais Actually reverted back because neither string nor regex version of using grep
works unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure that works? Grep uses case equality (===
), and afaik ”foo” === “f”
is false.
You could do it with two any?
/include?
lines, which would avoid both reflexes and the multi line postfix if
(which is what currently sticks out to me).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
”f === foo”
, even
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work with the regexes either unfortunately. The current version works so I don't think it's worth fiddling with it further.
You could do it with two any?/include? lines, which would avoid both reflexes and the multi line postfix if (which is what currently sticks out to me).
Unless it becomes very cluttered I think iterating once is better than twice.
Fix the code so we don't actually output `odeprecated` warnings for `HOMEBREW_CACHE`d or `.brew`d formulae.
Fix the code so we don't actually output
odeprecated
warnings forHOMEBREW_CACHE
d or.brew
d formulae.