Permalink
Browse files

Revert "Return HTTP 500 if git fails - not that Github cares about th…

…is at this point, but it seems more correct"

Looks like git (at least in version 1.6.6.1) outputs to STDERR even if no errors actually occur. It
was probably a bad idea to use the presence of data in STDERR as an indication of errors anyways.

This reverts commit 5f69896.
  • Loading branch information...
1 parent 5f69896 commit 417592f16316535e68ff3019a7b943c81bd7024e @koppen committed Mar 10, 2010
Showing with 7 additions and 24 deletions.
  1. +7 −11 app/controllers/github_hook_controller.rb
  2. +0 −13 test/functional/github_hook_controller_test.rb
@@ -21,13 +21,12 @@ def index
# Get updates from the Github repository
command = "cd '#{repository.url}' && git fetch origin && git reset --soft refs/remotes/origin/master"
- if exec(command)
- # Fetch the new changesets into Redmine
- repository.fetch_changesets
- render(:text => 'OK')
- else
- render(:text => 'Failed', :status => 500)
- end
+ exec(command)
+
+ # Fetch the new changesets into Redmine
+ repository.fetch_changesets
+
+ render(:text => 'OK')
end
private
@@ -39,15 +38,12 @@ def exec(command)
output = stdout.readlines.collect(&:strip)
errors = stderr.readlines.collect(&:strip)
- if errors.empty?
- return true
- else
+ unless errors.empty?
error_message = []
error_message << "Error occurred running git"
error_message += errors
error_message += output
logger.error error_message
- return false
end
end
@@ -147,17 +147,4 @@ def test_exec_should_not_log_errors_if_none_occurred
@controller.logger.expects(:error).never
do_post
end
-
- def test_should_return_500_if_git_has_errors
- @controller.expects(:exec).returns(false)
- do_post
- assert_response 500
- assert_equal 'Failed', @response.body
- end
-
- def test_should_not_import_changeset_if_git_has_errors
- @controller.expects(:exec).returns(false)
- @repository.expects(:fetch_changesets).never
- do_post
- end
end

0 comments on commit 417592f

Please sign in to comment.