Code review comment for lp:~fginther/cupstream2distro-config/fix-pbuilder-hooks

Revision history for this message
Martin Mrazik (mrazik) wrote :

I guess this is a matter of preference but I always get confused when I see just "True" somewhere:
34 + True)

Can you maybe change it to builder_job=True to improve the readability of the call?

This is not used anywhere (looks like a leftover from copy&paste):
135 + config = {
136 + 'hooks': 'parent-hook',
137 + 'configurations': {
138 + 'raring-amd64': {'node_label': 'pbuilder',
139 + 'hooks': 'child-only-hook'},
140 + 'raring-i386': {'node_label': 'pbuilder'}}}

Not sure if this is the error jenkins complains about but the following fails on my machine:

159 + self.assertEqual(count, 3)

I guess the following should be just +1? :
157 + count += 3

Other than it looks good.

review: Needs Fixing

« Back to merge proposal