Code review comment for lp:~milo/linaro-ci-dashboard/correct-chain-build

Revision history for this message
Stevan Radaković (stevanr) wrote :

On 09/14/2012 02:40 PM, Milo Casagrande wrote:
> On Fri, Sep 14, 2012 at 1:51 PM, Stevan Radaković
> <email address hidden> wrote:
>> Review: Needs Information
>> This needs some more work here in my opinion.
>> You also have a wrong description here. The build was scheduled for the chained loop, it's just that the results of the textfield loop build were not transferred to the android loop build.
> Yes, the point is that the values are not used at all at this stage.
> Even if it is scheduled, it is not building anything since we are not
> doing anything with those results.

Second part is still not correct. It actually builds the existing
AndroidLoop which is chained to this Textfield one.
Since you need to have the AndroidLoop in the system in order to chain
it, it has already some default set of options, and it can be build. The
problem still remains with not updating the chained loop with the
results of text field.

Regarding everything else, I didn't look at the code correctly.. I
somehow missed the lines

18 + for key, value in configuration.iteritems():
19 + if hasattr(self, key.lower()):
20 + setattr(self, key.lower(), value)
21 + else:

Everything else is clear.
Approving +1.

« Back to merge proposal