-
Notifications
You must be signed in to change notification settings - Fork 280
prepare-commit-message hook #520
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
Changes from all commits
70854f2
6004e53
225b7d1
8f2b48a
4aa26d6
ae2d529
5e338e7
9ae0dc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| require 'forwardable' | ||
|
|
||
| module Overcommit::Hook::PrepareCommitMsg | ||
| # Functionality common to all prepare-commit-msg hooks. | ||
| class Base < Overcommit::Hook::Base | ||
| extend Forwardable | ||
|
|
||
| def_delegators :@context, | ||
| :commit_message_filename, :commit_message_source, :commit, :lock | ||
|
|
||
| def modify_commit_message | ||
| raise 'This expects a block!' unless block_given? | ||
| # NOTE: this assumes all the hooks of the same type share the context's | ||
| # memory. If that's not the case, this won't work. | ||
| lock.synchronize do | ||
| contents = File.read(commit_message_filename) | ||
| File.open(commit_message_filename, 'w') do |f| | ||
| f << (yield contents) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| module Overcommit::Hook::PrepareCommitMsg | ||
| # Prepends the commit message with a message based on the branch name. | ||
| # It's possible to reference parts of the branch name through the captures in | ||
| # the `branch_pattern` regex. | ||
| class ReplaceBranch < Base | ||
| def run | ||
| return :pass unless !commit_message_source || | ||
| commit_message_source == :commit # NOTE: avoid 'merge' and 'rebase' | ||
| Overcommit::Utils.log.debug( | ||
| "Checking if '#{Overcommit::GitRepo.current_branch}' matches #{branch_pattern}" | ||
| ) | ||
| if branch_pattern.match(Overcommit::GitRepo.current_branch) | ||
| Overcommit::Utils.log.debug("Writing #{commit_message_filename} with #{new_template}") | ||
| modify_commit_message do |old_contents| | ||
| "#{new_template}\n#{old_contents}" | ||
| end | ||
| :pass | ||
| else | ||
| :warn | ||
| end | ||
| end | ||
|
|
||
| def new_template | ||
| @new_template ||= Overcommit::GitRepo.current_branch.gsub(branch_pattern, replacement_text) | ||
| end | ||
|
|
||
| def branch_pattern | ||
| @branch_pattern ||= | ||
| begin | ||
| pattern = config['branch_pattern'] | ||
| Regexp.new((pattern || '').empty? ? '\A.*\w+[-_](\d+).*\z' : pattern) | ||
| end | ||
| end | ||
|
|
||
| def replacement_text | ||
| @replacement_text ||= | ||
| begin | ||
| if File.exist?(replacement_text_config) | ||
| File.read(replacement_text_config) | ||
| else | ||
| replacement_text_config | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def replacement_text_config | ||
| @replacement_text_config ||= config['replacement_text'] | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| module Overcommit::HookContext | ||
| # Contains helpers related to contextual information used by prepare-commit-msg | ||
| # hooks. | ||
| class PrepareCommitMsg < Base | ||
| # Returns the name of the file that contains the commit log message | ||
| def commit_message_filename | ||
| @args[0] | ||
| end | ||
|
|
||
| # Returns the source of the commit message, and can be: message (if a -m or | ||
| # -F option was given); template (if a -t option was given or the | ||
| # configuration option commit.template is set); merge (if the commit is a | ||
| # merge or a .git/MERGE_MSG file exists); squash (if a .git/SQUASH_MSG file | ||
| # exists); or commit, followed by a commit SHA-1 (if a -c, -C or --amend | ||
| # option was given) | ||
| def commit_message_source | ||
| @args[1].to_sym if @args[1] | ||
| end | ||
|
|
||
| # Returns the commit's SHA-1. | ||
| # If commit_message_source is :commit, it's passed through the command-line. | ||
| def commit_message_source_ref | ||
| @args[2] || `git rev-parse HEAD` | ||
| end | ||
|
|
||
| # Lock for the pre_commit_message file. Should be shared by all | ||
| # prepare-commit-message hooks | ||
| def lock | ||
| @lock ||= Monitor.new | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| require 'spec_helper' | ||
| require 'overcommit/hook_context/prepare_commit_msg' | ||
|
|
||
| describe Overcommit::Hook::PrepareCommitMsg::Base do | ||
| let(:config) { Overcommit::ConfigurationLoader.default_configuration } | ||
| let(:context) { Overcommit::HookContext::PrepareCommitMsg.new(config, [], StringIO.new) } | ||
| let(:printer) { double('printer') } | ||
|
|
||
| context 'when multiple hooks run simultaneously' do | ||
| let(:hook_1) { described_class.new(config, context) } | ||
| let(:hook_2) { described_class.new(config, context) } | ||
|
|
||
| let(:tempfile) { 'test-prepare-commit-msg.txt' } | ||
|
|
||
| let(:initial_content) { "This is a test\n" } | ||
|
|
||
| before do | ||
| File.open(tempfile, 'w') do |f| | ||
| f << initial_content | ||
| end | ||
| end | ||
|
|
||
| after do | ||
| File.delete(tempfile) | ||
| end | ||
|
|
||
| it 'works well with concurrency' do | ||
| allow(context).to receive(:commit_message_filename).and_return(tempfile) | ||
| allow(hook_1).to receive(:run) do | ||
| hook_1.modify_commit_message do |contents| | ||
| "alpha\n" + contents | ||
| end | ||
| end | ||
| allow(hook_2).to receive(:run) do | ||
| hook_2.modify_commit_message do |contents| | ||
| contents + "bravo\n" | ||
| end | ||
| end | ||
| Thread.new { hook_1.run } | ||
| Thread.new { hook_2.run } | ||
| Thread.list.each { |t| t.join unless t == Thread.current } | ||
| expect(File.read(tempfile)).to match(/alpha\n#{initial_content}bravo\n/m) | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| require 'spec_helper' | ||
| require 'overcommit/hook_context/prepare_commit_msg' | ||
|
|
||
| describe Overcommit::Hook::PrepareCommitMsg::ReplaceBranch do | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file lacks
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine. As long as we have rigorous tests for the base class which validate the behavior of our commit message modifier helpers and the |
||
| let(:config) { Overcommit::ConfigurationLoader.default_configuration } | ||
| let(:context) do | ||
| Overcommit::HookContext::PrepareCommitMsg.new( | ||
| config, [prepare_commit_message_file, 'commit'], StringIO.new | ||
| ) | ||
| end | ||
|
|
||
| let(:prepare_commit_message_file) { 'prepare_commit_message_file.txt' } | ||
|
|
||
| subject(:hook) { described_class.new(config, context) } | ||
|
|
||
| before do | ||
| File.open(prepare_commit_message_file, 'w') | ||
| allow(Overcommit::Utils).to receive_message_chain(:log, :debug) | ||
| allow(Overcommit::GitRepo).to receive(:current_branch).and_return(new_head) | ||
| end | ||
|
|
||
| after do | ||
| File.delete(prepare_commit_message_file) unless ENV['APPVEYOR'] | ||
| end | ||
|
|
||
| let(:new_head) { 'userbeforeid-12345-branch-description' } | ||
|
|
||
| describe '#run' do | ||
| context 'when the checked out branch matches the pattern' do | ||
| it { is_expected.to pass } | ||
|
|
||
| context 'template contents' do | ||
| subject(:template) { hook.new_template } | ||
|
|
||
| before do | ||
| hook.stub(:replacement_text).and_return('Id is: \1') | ||
| end | ||
|
|
||
| it { is_expected.to eq('Id is: 12345') } | ||
| end | ||
| end | ||
|
|
||
| context 'when the checked out branch does not match the pattern' do | ||
| let(:new_head) { "this shouldn't match the default pattern" } | ||
|
|
||
| it { is_expected.to warn } | ||
| end | ||
| end | ||
|
|
||
| describe '#replacement_text' do | ||
| subject(:replacement_text) { hook.replacement_text } | ||
| let(:replacement_template_file) { 'valid_filename.txt' } | ||
| let(:replacement) { 'Id is: \1' } | ||
|
|
||
| context 'when the replacement text points to a valid filename' do | ||
| before do | ||
| hook.stub(:replacement_text_config).and_return(replacement_template_file) | ||
| File.stub(:exist?).and_return(true) | ||
| File.stub(:read).with(replacement_template_file).and_return(replacement) | ||
| end | ||
|
|
||
| describe 'it reads it as the replacement template' do | ||
| it { is_expected.to eq(replacement) } | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| require 'spec_helper' | ||
| require 'overcommit/hook_context/prepare_commit_msg' | ||
|
|
||
| describe Overcommit::HookContext::PrepareCommitMsg do | ||
| let(:config) { double('config') } | ||
| let(:args) { [commit_message_filename, commit_message_source] } | ||
| let(:commit_message_filename) { 'message-template.txt' } | ||
| let(:commit_message_source) { :file } | ||
| let(:commit) { 'SHA-1 here' } | ||
| let(:input) { double('input') } | ||
| let(:context) { described_class.new(config, args, input) } | ||
|
|
||
| describe '#commit_message_filename' do | ||
| subject { context.commit_message_filename } | ||
|
|
||
| it { should == commit_message_filename } | ||
| end | ||
|
|
||
| describe '#commit_message_source' do | ||
| subject { context.commit_message_source } | ||
|
|
||
| it { should == commit_message_source } | ||
| end | ||
| end |
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.
We need to be clear about the meaning of the value returned by this helper method. From my comment above, if the
commitis defined thenarg[2]will be the SHA-1 of the commit we're copying, but not the commit about to be created. I think this will be confusing and we should be explicit.Let's call this method
commit_message_source_ref. While it's not a true ref I think the wording helps emphasize that it's referring to a commit hash ref, and emphasizes the connection to thecommit_message_sourcehelper above.