audit: Make --display-failures-only the default for Casks

- Cask warnings are really noisy and numerous. Let's only show them if
  the user passes `--strict` or something implying `--strict`, like
  `--new-cask`.
- Additionally remove `display_passes` since we would like silence if
  nothing is wrong with the cask, the same as with formula audits.
This commit is contained in:
Issy Long 2023-03-29 00:40:46 +01:00
parent 931327df1f
commit a4e8f9e22b
No known key found for this signature in database
GPG Key ID: 8247C390DADC67D4
5 changed files with 33 additions and 63 deletions

View File

@ -109,14 +109,12 @@ module Cask
Formatter.error("failed") Formatter.error("failed")
elsif warnings? elsif warnings?
Formatter.warning("warning") Formatter.warning("warning")
else
Formatter.success("passed")
end end
end end
sig { params(include_passed: T::Boolean, include_warnings: T::Boolean).returns(T.nilable(String)) } sig { params(include_warnings: T::Boolean).returns(T.nilable(String)) }
def summary(include_passed: false, include_warnings: true) def summary(include_warnings: true)
return if success? && !include_passed return if success?
return if warnings? && !errors? && !include_warnings return if warnings? && !errors? && !include_warnings
summary = ["audit for #{cask}: #{result}"] summary = ["audit for #{cask}: #{result}"]

View File

@ -25,8 +25,6 @@ module Cask
quarantine: nil, quarantine: nil,
any_named_args: nil, any_named_args: nil,
language: nil, language: nil,
display_passes: nil,
display_failures_only: nil,
only: [], only: [],
except: [] except: []
) )
@ -40,8 +38,6 @@ module Cask
@audit_token_conflicts = audit_token_conflicts @audit_token_conflicts = audit_token_conflicts
@any_named_args = any_named_args @any_named_args = any_named_args
@language = language @language = language
@display_passes = display_passes
@display_failures_only = display_failures_only
@only = only @only = only
@except = except @except = except
end end
@ -63,7 +59,7 @@ module Cask
sample_languages.each_key do |l| sample_languages.each_key do |l|
audit = audit_languages(l) audit = audit_languages(l)
summary = audit.summary(include_passed: output_passed?, include_warnings: output_warnings?) summary = audit.summary(include_warnings: output_warnings?)
if summary.present? && output_summary?(audit) if summary.present? && output_summary?(audit)
ohai "Auditing language: #{l.map { |lang| "'#{lang}'" }.to_sentence}" if output_summary? ohai "Auditing language: #{l.map { |lang| "'#{lang}'" }.to_sentence}" if output_summary?
puts summary puts summary
@ -73,7 +69,7 @@ module Cask
end end
else else
audit = audit_cask_instance(cask) audit = audit_cask_instance(cask)
summary = audit.summary(include_passed: output_passed?, include_warnings: output_warnings?) summary = audit.summary(include_warnings: output_warnings?)
puts summary if summary.present? && output_summary?(audit) puts summary if summary.present? && output_summary?(audit)
warnings += audit.warnings warnings += audit.warnings
errors += audit.errors errors += audit.errors
@ -92,17 +88,8 @@ module Cask
audit.errors? audit.errors?
end end
def output_passed?
return false if @display_failures_only.present?
return true if @display_passes.present?
false
end
def output_warnings? def output_warnings?
return false if @display_failures_only.present? @new_cask.present? || @audit_strict.present?
true
end end
def audit_languages(languages) def audit_languages(languages)

View File

@ -30,8 +30,6 @@ module Cask
description: "Run various additional style checks to determine if a new cask is eligible " \ description: "Run various additional style checks to determine if a new cask is eligible " \
"for Homebrew. This should be used when creating new casks and implies " \ "for Homebrew. This should be used when creating new casks and implies " \
"`--strict` and `--online`" "`--strict` and `--online`"
switch "--display-failures-only",
description: "Only display casks that fail the audit. This is the default for formulae."
end end
end end
@ -49,19 +47,17 @@ module Cask
results = self.class.audit_casks( results = self.class.audit_casks(
*casks, *casks,
download: args.download?, download: args.download?,
online: args.online?, online: args.online?,
strict: args.strict?, strict: args.strict?,
signing: args.signing?, signing: args.signing?,
new_cask: args.new_cask?, new_cask: args.new_cask?,
token_conflicts: args.token_conflicts?, token_conflicts: args.token_conflicts?,
quarantine: args.quarantine?, quarantine: args.quarantine?,
any_named_args: any_named_args, any_named_args: any_named_args,
language: args.language, language: args.language,
display_passes: args.verbose? || args.named.count == 1, only: [],
display_failures_only: args.display_failures_only?, except: [],
only: [],
except: [],
) )
failed_casks = results.reject { |_, result| result[:errors].empty? }.map(&:first) failed_casks = results.reject { |_, result| result[:errors].empty? }.map(&:first)
@ -81,8 +77,6 @@ module Cask
quarantine:, quarantine:,
any_named_args:, any_named_args:,
language:, language:,
display_passes:,
display_failures_only:,
only:, only:,
except: except:
) )
@ -96,7 +90,6 @@ module Cask
quarantine: quarantine, quarantine: quarantine,
language: language, language: language,
any_named_args: any_named_args, any_named_args: any_named_args,
display_passes: display_passes,
display_failures_only: display_failures_only, display_failures_only: display_failures_only,
only: only, only: only,
except: except, except: except,

View File

@ -65,7 +65,7 @@ module Homebrew
description: "Prefix every line of output with the file or formula name being audited, to " \ description: "Prefix every line of output with the file or formula name being audited, to " \
"make output easy to grep." "make output easy to grep."
switch "--display-failures-only", switch "--display-failures-only",
description: "Only display casks that fail the audit. This is the default for formulae." description: "Only display casks that fail the audit. This is the default for formulae and casks."
switch "--skip-style", switch "--skip-style",
description: "Skip running non-RuboCop style checks. Useful if you plan on running " \ description: "Skip running non-RuboCop style checks. Useful if you plan on running " \
"`brew style` separately. Enabled by default unless a formula is specified by name." "`brew style` separately. Enabled by default unless a formula is specified by name."
@ -242,24 +242,26 @@ module Homebrew
require "cask/cmd/abstract_command" require "cask/cmd/abstract_command"
require "cask/cmd/audit" require "cask/cmd/audit"
if args.display_failures_only?
odeprecated "`brew audit <cask> --display-failures-only`", "`brew audit <cask>` without the argument"
end
# For switches, we add `|| nil` so that `nil` will be passed instead of `false` if they aren't set. # For switches, we add `|| nil` so that `nil` will be passed instead of `false` if they aren't set.
# This way, we can distinguish between "not set" and "set to false". # This way, we can distinguish between "not set" and "set to false".
Cask::Cmd::Audit.audit_casks( Cask::Cmd::Audit.audit_casks(
*audit_casks, *audit_casks,
download: nil, download: nil,
# No need for `|| nil` for `--[no-]signing` because boolean switches are already `nil` if not passed # No need for `|| nil` for `--[no-]signing` because boolean switches are already `nil` if not passed
signing: args.signing?, signing: args.signing?,
online: args.online? || nil, online: args.online? || nil,
strict: args.strict? || nil, strict: args.strict? || nil,
new_cask: args.new_cask? || nil, new_cask: args.new_cask? || nil,
token_conflicts: args.token_conflicts? || nil, token_conflicts: args.token_conflicts? || nil,
quarantine: nil, quarantine: nil,
any_named_args: !no_named_args, any_named_args: !no_named_args,
language: nil, language: nil,
display_passes: args.verbose? || args.named.count == 1, only: args.only,
display_failures_only: args.display_failures_only?, except: args.except,
only: args.only,
except: args.except,
) )
end end

View File

@ -25,7 +25,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_new_cask: false, quarantine: true, any_named_args: true, audit_new_cask: false, quarantine: true, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -40,7 +39,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_new_cask: false, quarantine: true, any_named_args: true, audit_new_cask: false, quarantine: true, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -54,7 +52,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_download: true, audit_new_cask: false, quarantine: true, any_named_args: true, audit_download: true, audit_new_cask: false, quarantine: true, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -68,7 +65,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_token_conflicts: true, audit_new_cask: false, quarantine: true, any_named_args: true, audit_token_conflicts: true, audit_new_cask: false, quarantine: true, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -82,7 +78,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_strict: true, audit_new_cask: false, quarantine: true, any_named_args: true, audit_strict: true, audit_new_cask: false, quarantine: true, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -96,7 +91,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_online: true, audit_new_cask: false, quarantine: true, any_named_args: true, audit_online: true, audit_new_cask: false, quarantine: true, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -110,7 +104,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_new_cask: true, quarantine: true, any_named_args: true, audit_new_cask: true, quarantine: true, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -124,7 +117,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_new_cask: false, quarantine: true, language: ["de-AT"], any_named_args: true, audit_new_cask: false, quarantine: true, language: ["de-AT"], any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -138,7 +130,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_new_cask: false, quarantine: false, any_named_args: true, audit_new_cask: false, quarantine: false, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)
@ -154,7 +145,6 @@ describe Cask::Cmd::Audit, :cask do
.with( .with(
cask, cask,
audit_new_cask: false, quarantine: false, any_named_args: true, audit_new_cask: false, quarantine: false, any_named_args: true,
display_failures_only: false, display_passes: true,
only: [], except: [] only: [], except: []
) )
.and_return(result) .and_return(result)