From a4e8f9e22b78c740cb0df0d3d8e70ff6c0bd6036 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Wed, 29 Mar 2023 00:40:46 +0100 Subject: [PATCH] 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. --- Library/Homebrew/cask/audit.rb | 8 ++---- Library/Homebrew/cask/auditor.rb | 19 ++----------- Library/Homebrew/cask/cmd/audit.rb | 29 +++++++------------ Library/Homebrew/dev-cmd/audit.rb | 30 +++++++++++--------- Library/Homebrew/test/cask/cmd/audit_spec.rb | 10 ------- 5 files changed, 33 insertions(+), 63 deletions(-) diff --git a/Library/Homebrew/cask/audit.rb b/Library/Homebrew/cask/audit.rb index 8b65aca250..c9d299e3e9 100644 --- a/Library/Homebrew/cask/audit.rb +++ b/Library/Homebrew/cask/audit.rb @@ -109,14 +109,12 @@ module Cask Formatter.error("failed") elsif warnings? Formatter.warning("warning") - else - Formatter.success("passed") end end - sig { params(include_passed: T::Boolean, include_warnings: T::Boolean).returns(T.nilable(String)) } - def summary(include_passed: false, include_warnings: true) - return if success? && !include_passed + sig { params(include_warnings: T::Boolean).returns(T.nilable(String)) } + def summary(include_warnings: true) + return if success? return if warnings? && !errors? && !include_warnings summary = ["audit for #{cask}: #{result}"] diff --git a/Library/Homebrew/cask/auditor.rb b/Library/Homebrew/cask/auditor.rb index c4f93896dc..cae71e5d13 100644 --- a/Library/Homebrew/cask/auditor.rb +++ b/Library/Homebrew/cask/auditor.rb @@ -25,8 +25,6 @@ module Cask quarantine: nil, any_named_args: nil, language: nil, - display_passes: nil, - display_failures_only: nil, only: [], except: [] ) @@ -40,8 +38,6 @@ module Cask @audit_token_conflicts = audit_token_conflicts @any_named_args = any_named_args @language = language - @display_passes = display_passes - @display_failures_only = display_failures_only @only = only @except = except end @@ -63,7 +59,7 @@ module Cask sample_languages.each_key do |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) ohai "Auditing language: #{l.map { |lang| "'#{lang}'" }.to_sentence}" if output_summary? puts summary @@ -73,7 +69,7 @@ module Cask end else 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) warnings += audit.warnings errors += audit.errors @@ -92,17 +88,8 @@ module Cask audit.errors? end - def output_passed? - return false if @display_failures_only.present? - return true if @display_passes.present? - - false - end - def output_warnings? - return false if @display_failures_only.present? - - true + @new_cask.present? || @audit_strict.present? end def audit_languages(languages) diff --git a/Library/Homebrew/cask/cmd/audit.rb b/Library/Homebrew/cask/cmd/audit.rb index 35179c3455..0c65d24b22 100644 --- a/Library/Homebrew/cask/cmd/audit.rb +++ b/Library/Homebrew/cask/cmd/audit.rb @@ -30,8 +30,6 @@ module Cask 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 " \ "`--strict` and `--online`" - switch "--display-failures-only", - description: "Only display casks that fail the audit. This is the default for formulae." end end @@ -49,19 +47,17 @@ module Cask results = self.class.audit_casks( *casks, - download: args.download?, - online: args.online?, - strict: args.strict?, - signing: args.signing?, - new_cask: args.new_cask?, - token_conflicts: args.token_conflicts?, - quarantine: args.quarantine?, - any_named_args: any_named_args, - language: args.language, - display_passes: args.verbose? || args.named.count == 1, - display_failures_only: args.display_failures_only?, - only: [], - except: [], + download: args.download?, + online: args.online?, + strict: args.strict?, + signing: args.signing?, + new_cask: args.new_cask?, + token_conflicts: args.token_conflicts?, + quarantine: args.quarantine?, + any_named_args: any_named_args, + language: args.language, + only: [], + except: [], ) failed_casks = results.reject { |_, result| result[:errors].empty? }.map(&:first) @@ -81,8 +77,6 @@ module Cask quarantine:, any_named_args:, language:, - display_passes:, - display_failures_only:, only:, except: ) @@ -96,7 +90,6 @@ module Cask quarantine: quarantine, language: language, any_named_args: any_named_args, - display_passes: display_passes, display_failures_only: display_failures_only, only: only, except: except, diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index a481432293..7cb204f9cb 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -65,7 +65,7 @@ module Homebrew description: "Prefix every line of output with the file or formula name being audited, to " \ "make output easy to grep." 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", 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." @@ -242,24 +242,26 @@ module Homebrew require "cask/cmd/abstract_command" require "cask/cmd/audit" + if args.display_failures_only? + odeprecated "`brew audit --display-failures-only`", "`brew audit ` without the argument" + end + # 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". Cask::Cmd::Audit.audit_casks( *audit_casks, - download: nil, + download: nil, # No need for `|| nil` for `--[no-]signing` because boolean switches are already `nil` if not passed - signing: args.signing?, - online: args.online? || nil, - strict: args.strict? || nil, - new_cask: args.new_cask? || nil, - token_conflicts: args.token_conflicts? || nil, - quarantine: nil, - any_named_args: !no_named_args, - language: nil, - display_passes: args.verbose? || args.named.count == 1, - display_failures_only: args.display_failures_only?, - only: args.only, - except: args.except, + signing: args.signing?, + online: args.online? || nil, + strict: args.strict? || nil, + new_cask: args.new_cask? || nil, + token_conflicts: args.token_conflicts? || nil, + quarantine: nil, + any_named_args: !no_named_args, + language: nil, + only: args.only, + except: args.except, ) end diff --git a/Library/Homebrew/test/cask/cmd/audit_spec.rb b/Library/Homebrew/test/cask/cmd/audit_spec.rb index cf15e3939a..12728b4777 100644 --- a/Library/Homebrew/test/cask/cmd/audit_spec.rb +++ b/Library/Homebrew/test/cask/cmd/audit_spec.rb @@ -25,7 +25,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_new_cask: false, quarantine: true, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -40,7 +39,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_new_cask: false, quarantine: true, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -54,7 +52,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_download: true, audit_new_cask: false, quarantine: true, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -68,7 +65,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_token_conflicts: true, audit_new_cask: false, quarantine: true, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -82,7 +78,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_strict: true, audit_new_cask: false, quarantine: true, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -96,7 +91,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_online: true, audit_new_cask: false, quarantine: true, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -110,7 +104,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_new_cask: true, quarantine: true, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -124,7 +117,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_new_cask: false, quarantine: true, language: ["de-AT"], any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -138,7 +130,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_new_cask: false, quarantine: false, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result) @@ -154,7 +145,6 @@ describe Cask::Cmd::Audit, :cask do .with( cask, audit_new_cask: false, quarantine: false, any_named_args: true, - display_failures_only: false, display_passes: true, only: [], except: [] ) .and_return(result)