Merge lp:~matsubara/tarmac/tarmac-bug-418823 into lp:tarmac

Proposed by Diogo Matsubara
Status: Merged
Approved by: Paul Hummer
Approved revision: no longer in the source branch.
Merge reported by: Diogo Matsubara
Merged at revision: not available
Proposed branch: lp:~matsubara/tarmac/tarmac-bug-418823
Merge into: lp:tarmac
Diff against target: None lines
To merge this branch: bzr merge lp:~matsubara/tarmac/tarmac-bug-418823
Reviewer Review Type Date Requested Status
Paul Hummer Approve
Review via email: mp+10818@code.launchpad.net

Commit message

Fix bug 418823 (Errors in the pre-commit hook should be logged) and typo fix in docs/configuration.txt.

To post a comment you must log in.
Revision history for this message
Diogo Matsubara (matsubara) wrote :

Hi there,

this branch adds logging when [pre,post] commit hooks fail (bug 418823), fix the spam-merge-proposal-on-test-failure issue by using the setStatus() api properly and fix a typo in docs/configuration.txt

Also adds a XXX to run_test.py where raise HookFailed() is called. That line will always fail with an IndexError as it's passing the wrong parameter to HookFailed. Not sure how to fix that, though. Suggestions welcome.

147. By dobey

Adding reviewer metadata now checks to make sure that the reviewer is an official reviewer. (Rodney Dawes)

Revision history for this message
Paul Hummer (rockstar) wrote :

> Also adds a XXX to run_test.py where raise HookFailed() is called. That line
> will always fail with an IndexError as it's passing the wrong parameter to
> HookFailed. Not sure how to fix that, though. Suggestions welcome.

Could you please file a bug on this, and cite it in the XXX comment. I'll take a look at this soon. Otherwise, I'm happy with this branch.

review: Approve
148. By dobey

Better format for the merge failure comment message. (Rodney Dawes)

Revision history for this message
Diogo Matsubara (matsubara) wrote :

> > Also adds a XXX to run_test.py where raise HookFailed() is called. That line
> > will always fail with an IndexError as it's passing the wrong parameter to
> > HookFailed. Not sure how to fix that, though. Suggestions welcome.
>
> Could you please file a bug on this, and cite it in the XXX comment. I'll
> take a look at this soon. Otherwise, I'm happy with this branch.

Filed bug 424466 and added reference to it in the XXX comment. I've also solved the conflict with Rodney's branch. Thanks for reviewing.

149. By Diogo Matsubara

Fix bug 418823 (Errors in the pre-commit hook should be logged) and typo fix in docs/configuration.txt

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'docs/configuration.txt'
2--- docs/configuration.txt 2009-07-11 05:01:35 +0000
3+++ docs/configuration.txt 2009-08-27 20:57:58 +0000
4@@ -33,8 +33,8 @@
5 Tarmac does its best to only have to fetch the WorkingTree of the trunk
6 branch once, but sometimes this can have issues. Tarmac has support for
7 specifying where a user may have already placed a WorkingTree of the target
8-branch. In order to use this, simply set the ``tree_dir`` config settisg as
9-shown below::
10+branch. In order to use this, simply set the ``tree_dir`` config setting as
11+shown below:
12
13 [tarmac]
14 # Configuration for the Tarmac project
15
16=== modified file 'tarmac/bin.py'
17--- tarmac/bin.py 2009-07-15 02:57:36 +0000
18+++ tarmac/bin.py 2009-08-27 20:57:58 +0000
19@@ -244,7 +244,9 @@
20 self.options, self.configuration, candidate, trunk)
21
22 except Exception, e:
23- print e
24+ message = "Oops! Tarmac hooks failed:\n %s" % e
25+ self.logger.error(message)
26+ print message
27
28 trunk.cleanup()
29
30
31=== modified file 'tarmac/plugins/runtest.py'
32--- tarmac/plugins/runtest.py 2009-08-10 19:12:24 +0000
33+++ tarmac/plugins/runtest.py 2009-08-27 20:57:58 +0000
34@@ -62,6 +62,10 @@
35
36 else:
37 self.do_failed(stdout_value, stderr_value)
38+ #XXX matsubara: this line will always fail with
39+ # IndexError: string index out of range. HookFailed expects a
40+ # a (exc_type, exc_value, exc_tb) object. Maybe use
41+ # sys.exc_info() here?
42 raise HookFailed('test', 'runtest', '')
43
44 def do_failed(self, stdout_value, stderr_value):
45@@ -75,7 +79,7 @@
46 comment = u'\n'.join([stdout_value, stderr_value])
47 self.candidate.createComment(subject="Failed test command",
48 content=comment)
49- self.candidate.setStatus(u'Work in progress')
50+ self.candidate.setStatus(status=u'Needs review')
51 self.candidate.lp_save()
52
53

Subscribers

People subscribed via source and target branches