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
Scope HOMEBREW_NO_INSTALL_FROM_API
to core formulae in brew audit
#14793
Conversation
Review period skipped due to |
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.
Looks good, thanks for this @Bo98! Some nits but fine if this merges as-is if you disagree.
Library/Homebrew/utils.rb
Outdated
@@ -317,6 +317,12 @@ def with_custom_locale(locale, &block) | |||
with_env(LC_ALL: locale, &block) | |||
end | |||
|
|||
def without_api(&block) |
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.
Can this go into Library/Homebrew/dev-cmd/audit.rb
as it's only used there?
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 would not be surprised if it is later used elsewhere e.g. brew test
. But I guess I can shift it back to utils.rb
later?
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.
@Bo98 Yup, or ideally something like Utils::API.without
Library/Homebrew/dev-cmd/audit.rb
Outdated
odeprecated "brew audit [path ...]", | ||
"brew audit [name ...]" | ||
end | ||
audit_formulae, audit_casks = without_api do |
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.
Comments explaining why this is needed would be nice here.
@@ -209,8 +211,12 @@ def audit | |||
display_cop_names: args.display_cop_names?, | |||
}.compact | |||
|
|||
fa = FormulaAuditor.new(f, **options) | |||
fa.audit | |||
audit_proc = proc { FormulaAuditor.new(f, **options).tap(&:audit) } |
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.
Comments explaining why this is needed would be nice here.
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.
Looks great, thanks for adjusting!
See #14701