Code review comment for lp:~pfalcon/linaro-ci-dashboard/kernel_chain

Revision history for this message
Paul Sokolovsky (pfalcon) wrote :

Milo:

> Is it possible to be more greedy here? Like "*.img.*", so to avoid
file suffix, or there might be other file types that we do not want?
Minor suggestion though...

Well, if you looked carefully, what I did to that line was adding "MANIFEST" file import. The original line likely was added by yourself while working on Android loop. But as it apparently was taken from a build config as set up be me, here's reply to the very question:

What about "1.img.something.completely.different" file? So yes, it's grounded conservatism imho. Of course, if there're good reasons to change that, we can.

>This is more a personal thing than anything else: why not having only
a single exit point? Just using a single return statement.

Single statement of course would be possible, but that would program transformation skewing the original idea of the code. It's because 2 branches have different semantics: one executes if data is valid and returns expected processed data, while another executes when data invalid and returns "data invalid" token. To make the point more clear, in the next incarnation of this function, return in "data invalid" branch may become raise.

« Back to merge proposal