Skip to content
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.

Sign up for GitHub

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

Jump to bottom

utils: reduce odeprecated warnings. #3725

Merged
merged 1 commit into from
Jan 24, 2018
Merged

utils: reduce odeprecated warnings. #3725

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions Library/Homebrew/utils.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,19 @@ def odeprecated(method, replacement = nil, disable: false, disable_on: nil, call
# - Location of caller of deprecated method (if all else fails).
backtrace = caller
tap_message = nil

# 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
Copy link
Contributor

@alyssais alyssais Jan 23, 2018 •
edited
Loading

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/})

Copy link
Member Author

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)

Copy link
Member Author

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.

Copy link
Contributor

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

”f === foo”, even

Copy link
Member Author

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.


caller_message = backtrace.detect do |line|
next unless line =~ %r{^#{Regexp.escape(HOMEBREW_LIBRARY)}/Taps/([^/]+/[^/]+)/}
tap = Tap.fetch Regexp.last_match(1)
tap_message = "\nPlease report this to the #{tap} tap!"
true
end
caller_message ||= backtrace.detect do |line|
# Don't throw deprecations at all for cached or .brew formulae.
next false if line.include?(HOMEBREW_CACHE)
next false if line.include?("/.brew/")
!line.start_with?("#{HOMEBREW_LIBRARY_PATH}/compat/")
end
caller_message ||= backtrace[1]
Expand Down