From dc50788d3c6e710231e1efa6bfba9a16e334fae5 Mon Sep 17 00:00:00 2001 From: Timo Webler Date: Fri, 31 Jan 2020 10:26:48 +0100 Subject: [PATCH 1/6] Increase performance of PhpCs pre commit hook --- lib/overcommit/hook/pre_commit/php_cs.rb | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/overcommit/hook/pre_commit/php_cs.rb b/lib/overcommit/hook/pre_commit/php_cs.rb index 6204c8dd..8e837406 100644 --- a/lib/overcommit/hook/pre_commit/php_cs.rb +++ b/lib/overcommit/hook/pre_commit/php_cs.rb @@ -12,21 +12,14 @@ class PhpCs < Base def run messages = [] - applicable_files.each do |file| - result = execute(command, args: [file]) - if result.status - rows = result.stdout.split("\n") - - # Discard the csv header - rows.shift - - # Push each of the errors in the particular file into the array - rows.map do |row| - messages << row - end - end + result = execute(command, args: applicable_files) + if result.status + messages = result.stdout.split("\n") + # Discard the csv header + messages.shift end + return :fail if messages.empty? && !result.success? return :pass if messages.empty? parse_messages(messages) From 16eded31008dcb050d36534c970f3c28994afb7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Povilas=20Jur=C4=8Dys?= Date: Sun, 8 Mar 2020 06:01:35 +0200 Subject: [PATCH 2/6] Add pronto to pre_push hooks (#706) --- .gitignore | 2 + README.md | 1 + config/default.yml | 7 +++ lib/overcommit/hook/pre_commit/pronto.rb | 17 ++----- lib/overcommit/hook/pre_push/base.rb | 5 ++ lib/overcommit/hook/pre_push/pronto.rb | 12 +++++ lib/overcommit/hook/shared/pronto.rb | 21 ++++++++ spec/overcommit/hook/pre_push/pronto_spec.rb | 53 ++++++++++++++++++++ 8 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 lib/overcommit/hook/pre_push/pronto.rb create mode 100644 lib/overcommit/hook/shared/pronto.rb create mode 100644 spec/overcommit/hook/pre_push/pronto_spec.rb diff --git a/.gitignore b/.gitignore index 2bb7f841..7ea8f1f7 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ coverage/ pkg/ .bundle .idea +.history/ +.vscode/ diff --git a/README.md b/README.md index f1076df6..7f996177 100644 --- a/README.md +++ b/README.md @@ -582,6 +582,7 @@ aborted. * [Brakeman](lib/overcommit/hook/pre_push/brakeman.rb) * [Minitest](lib/overcommit/hook/pre_push/minitest.rb) * [PhpUnit](lib/overcommit/hook/pre_push/php_unit.rb) +* [Pronto](lib/overcommit/hook/pre_push/pronto.rb) * [ProtectedBranches](lib/overcommit/hook/pre_push/protected_branches.rb) * [Pytest](lib/overcommit/hook/pre_push/pytest.rb) * [PythonNose](lib/overcommit/hook/pre_push/python_nose.rb) diff --git a/config/default.yml b/config/default.yml index 5a6887f4..74503e8e 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1297,6 +1297,13 @@ PrePush: flags: ['--bootstrap', 'vendor/autoload.php', 'tests'] install_command: 'composer require --dev phpunit/phpunit' + Pronto: + enabled: false + description: 'Analyzing with pronto' + required_executable: 'pronto' + install_command: 'gem install pronto' + flags: ['run', '--exit-code'] + ProtectedBranches: enabled: false description: 'Check for illegal pushes to protected branches' diff --git a/lib/overcommit/hook/pre_commit/pronto.rb b/lib/overcommit/hook/pre_commit/pronto.rb index da016e8f..664b4ef8 100644 --- a/lib/overcommit/hook/pre_commit/pronto.rb +++ b/lib/overcommit/hook/pre_commit/pronto.rb @@ -1,23 +1,12 @@ # frozen_string_literal: true +require 'overcommit/hook/shared/pronto' + module Overcommit::Hook::PreCommit # Runs `pronto` # # @see https://github.com/mmozuras/pronto class Pronto < Base - MESSAGE_TYPE_CATEGORIZER = lambda do |type| - type.include?('E') ? :error : :warning - end - - def run - result = execute(command) - return :pass if result.success? - - extract_messages( - result.stdout.split("\n"), - /^(?(?:\w:)?[^:]+):(?\d+) (?[^ ]+)/, - MESSAGE_TYPE_CATEGORIZER, - ) - end + include Overcommit::Hook::Shared::Pronto end end diff --git a/lib/overcommit/hook/pre_push/base.rb b/lib/overcommit/hook/pre_push/base.rb index 72c89d0c..35e2b3a8 100644 --- a/lib/overcommit/hook/pre_push/base.rb +++ b/lib/overcommit/hook/pre_push/base.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'forwardable' +require 'overcommit/utils/messages_utils' module Overcommit::Hook::PrePush # Functionality common to all pre-push hooks. @@ -17,6 +18,10 @@ def run? private + def extract_messages(*args) + Overcommit::Utils::MessagesUtils.extract_messages(*args) + end + def exclude_remotes @config['exclude_remotes'] || [] end diff --git a/lib/overcommit/hook/pre_push/pronto.rb b/lib/overcommit/hook/pre_push/pronto.rb new file mode 100644 index 00000000..93a0724e --- /dev/null +++ b/lib/overcommit/hook/pre_push/pronto.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'overcommit/hook/shared/pronto' + +module Overcommit::Hook::PrePush + # Runs `pronto` + # + # @see https://github.com/mmozuras/pronto + class Pronto < Base + include Overcommit::Hook::Shared::Pronto + end +end diff --git a/lib/overcommit/hook/shared/pronto.rb b/lib/overcommit/hook/shared/pronto.rb new file mode 100644 index 00000000..b1d71442 --- /dev/null +++ b/lib/overcommit/hook/shared/pronto.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Overcommit::Hook::Shared + # Shared code used by all Pronto hooks. Runs pronto linter. + module Pronto + MESSAGE_TYPE_CATEGORIZER = lambda do |type| + type.include?('E') ? :error : :warning + end + + def run + result = execute(command) + return :pass if result.success? + + extract_messages( + result.stdout.split("\n"), + /^(?(?:\w:)?[^:]+):(?\d+) (?[^ ]+)/, + MESSAGE_TYPE_CATEGORIZER, + ) + end + end +end diff --git a/spec/overcommit/hook/pre_push/pronto_spec.rb b/spec/overcommit/hook/pre_push/pronto_spec.rb new file mode 100644 index 00000000..2851a860 --- /dev/null +++ b/spec/overcommit/hook/pre_push/pronto_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Overcommit::Hook::PrePush::Pronto do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + subject { described_class.new(config, context) } + + before do + subject.stub(:applicable_files).and_return(%w[file1.rb file2.rb]) + end + + context 'when pronto exits successfully' do + before do + result = double('result') + result.stub(:success?).and_return(true) + subject.stub(:execute).and_return(result) + end + + it { should pass } + end + + context 'when pronto exits unsucessfully' do + let(:result) { double('result') } + + before do + result.stub(:success?).and_return(false) + subject.stub(:execute).and_return(result) + end + + context 'and it reports an error' do + before do + result.stub(:stdout).and_return([ + 'file2.rb:10 E: IDENTICAL code found in :iter.', + ].join("\n")) + end + + it { should fail_hook } + end + + context 'and it reports a warning' do + before do + result.stub(:stdout).and_return([ + 'file1.rb:12 W: Line is too long. [107/80]', + 'file2.rb:14 I: Prefer single-quoted strings' + ].join("\n")) + end + + it { should warn } + end + end +end From d9e499874ace66203d063207c7f6f9d8dc543796 Mon Sep 17 00:00:00 2001 From: Cole Mackenzie Date: Tue, 10 Mar 2020 21:18:51 -0700 Subject: [PATCH 3/6] Removing extra newline in replacement string (#709) Co-authored-by: Cole Mackenzie --- lib/overcommit/hook/prepare_commit_msg/replace_branch.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb b/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb index ce490163..c64f4c22 100644 --- a/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb +++ b/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb @@ -17,7 +17,7 @@ def run Overcommit::Utils.log.debug("Writing #{commit_message_filename} with #{new_template}") modify_commit_message do |old_contents| - "#{new_template}\n#{old_contents}" + "#{new_template} #{old_contents}" end :pass From a15edd2df9f3c3626112a84f92c5cf8a6b39d3d3 Mon Sep 17 00:00:00 2001 From: Shane da Silva Date: Sun, 5 Apr 2020 18:05:25 -0700 Subject: [PATCH 4/6] Add note about checking stash if hook interrupted Closes #711 --- lib/overcommit/printer.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/overcommit/printer.rb b/lib/overcommit/printer.rb index aebb8326..02d0995d 100644 --- a/lib/overcommit/printer.rb +++ b/lib/overcommit/printer.rb @@ -51,6 +51,7 @@ def interrupt_triggered def run_interrupted log.newline log.warning '⚠ Hook run interrupted by user' + log.warning '⚠ If files appear modified/missing, check your stash to recover them' log.newline end From 9a4028ec3470a634418f45e4a056e61f7f5ccb01 Mon Sep 17 00:00:00 2001 From: Kevin Deisz Date: Fri, 24 Apr 2020 01:30:07 -0400 Subject: [PATCH 5/6] Add skip_if hook option (#715) Skip hook with `skip_if` configuration if defined. --- README.md | 1 + lib/overcommit/hook/base.rb | 3 +- spec/overcommit/hook/base_spec.rb | 65 +++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 7f996177..af718032 100644 --- a/README.md +++ b/README.md @@ -241,6 +241,7 @@ Option | Description `processors` | The number of processing units to reserve for this hook. This does not reserve CPUs, but indicates that out of the total number of possible concurrent hooks allowed by the global `concurrency` option, this hook requires the specified number. Thus in the typical case where `concurrency` is set to the number of available cores (default), and you have a hook that executes an application which itself creates 2 threads (or is otherwise scheduled on 2 cores), you can indicate that Overcommit should allocate 2 `processors` to the hook. Ideally this means your hooks won't put undue load on your available cores. `install_command` | Command the user can run to install the `required_executable` (or alternately the specified `required_libraries`). This is intended for documentation purposes, as Overcommit does not install software on your behalf since there are too many edge cases where such behavior would result in incorrectly configured installations (e.g. installing a Python package in the global package space instead of in a virtual environment). `skip_file_checkout` | Whether to skip this hook for file checkouts (e.g. `git checkout some-ref -- file`). Only applicable to `PostCheckout` hooks. +`skip_if` | Array of arguments to be executed to determine whether or not the hook should run. For example, setting this to a value of `['bash', '-c', '! which my-executable']` would allow you to skip running this hook if `my-executable` was not in the bin path. In addition to the built-in configuration options, each hook can expose its own unique configuration options. The `AuthorEmail` hook, for example, allows diff --git a/lib/overcommit/hook/base.rb b/lib/overcommit/hook/base.rb index 4f48d770..37696d08 100644 --- a/lib/overcommit/hook/base.rb +++ b/lib/overcommit/hook/base.rb @@ -84,7 +84,8 @@ def excluded? end def skip? - @config['skip'] + @config['skip'] || + (@config['skip_if'] ? execute(@config['skip_if']).success? : false) end def run? diff --git a/spec/overcommit/hook/base_spec.rb b/spec/overcommit/hook/base_spec.rb index 8a0e5d46..0c8c7af7 100644 --- a/spec/overcommit/hook/base_spec.rb +++ b/spec/overcommit/hook/base_spec.rb @@ -122,4 +122,69 @@ end end end + + context '#skip?' do + before do + config.stub(:for_hook).and_return(hook_config) + end + + subject { hook.skip? } + + context 'with skip_if not specified' do + let(:hook_config) do + { 'skip' => skip } + end + + context 'with skip true' do + let(:skip) { true } + + it { subject.should == true } + end + + context 'with skip false' do + let(:skip) { false } + + it { subject.should == false } + end + end + + context 'with skip_if specified' do + before do + result = Overcommit::Subprocess::Result.new(success ? 0 : 1, '', '') + allow(Overcommit::Utils).to receive(:execute).and_return(result) + end + + let(:hook_config) do + { 'skip' => skip, 'skip_if' => ['bash', '-c', '! which my-executable'] } + end + + context 'with skip true and skip_if returning true' do + let(:skip) { true } + let(:success) { true } + + it { subject.should == true } + end + + context 'with skip true and skip_if returning false' do + let(:skip) { true } + let(:success) { false } + + it { subject.should == true } + end + + context 'with skip false and skip_if returning true' do + let(:skip) { false } + let(:success) { true } + + it { subject.should == true } + end + + context 'with skip false and skip_if returning false' do + let(:skip) { false } + let(:success) { false } + + it { subject.should == false } + end + end + end end From a409887771182f9e1411d53b806fff025fe61851 Mon Sep 17 00:00:00 2001 From: Shane da Silva Date: Thu, 23 Apr 2020 22:34:57 -0700 Subject: [PATCH 6/6] Cut version 0.53.0 --- CHANGELOG.md | 8 ++++++++ lib/overcommit/version.rb | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c586f60..1711cb54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Overcommit Changelog +## 0.53.0 + +* Improve performance in `PhpCs` pre-commit hook +* Add `Pronto` pre-push hook +* Remove erroneous extra newline in replacement string for `ReplaceBranch` prepare-commit-msg hook +* Add note about potentially checking your stash when hook is interrupted +* Add support for skipping hooks based on command result using the `skip_if` option + ## 0.52.1 * Fix case where no standard input is provided to `pre-push` hooks diff --git a/lib/overcommit/version.rb b/lib/overcommit/version.rb index f6c31248..d4659a8b 100644 --- a/lib/overcommit/version.rb +++ b/lib/overcommit/version.rb @@ -2,5 +2,5 @@ # Defines the gem version. module Overcommit - VERSION = '0.52.1' + VERSION = '0.53.0' end