Code review comment for lp:~pwlars/lava-dispatcher/add-uboot-support

Revision history for this message
Paul Larson (pwlars) wrote :

> Just two comments. I don't think we should default to beagle in the
> initializer. It makes it possible to omit this argument (or worse, develop on
> beagle all the time) and produce buggy code by accident.
Yeah, for some things it's a reasonable default since we have a wide variety of boards that correspond to this hwpack. In this case though, it could be a bad assumption. I'll make it explicit.

> I would also create
> constants for names like beagle to make sure a simple typo will not be
> something hard to chase and debug. Right now it's not needed but I suspect
> this wil come back.
I'm not sure if I see how this would help. If I understand what you're getting at, in either case the failure would be pretty much the same - trying to reference an entry in the dict that does not exist, and getting nothing back to send to uboot?

Will check this in for now, and we can deal with that later if needed.

« Back to merge proposal