Code review comment for lp:~james-w/linaro-image-tools/add-other-relations-to-packages-file

Revision history for this message
James Westby (james-w) wrote :

On Tue, 14 Sep 2010 19:47:43 -0000, Zygmunt Krynicki <email address hidden> wrote:
> Review: Needs Fixing
> You seem to have changed FetchedPackage.__eq__ but did not change
> __hash__ (it still refers to the old set of fields).

Because content() was never in the __hash__ calculation.

> In addition you should not define __eq__ but __cmp__ unless you are
> prepared to define __ne__. Moreover you should not define __hash__ as
> your instances are not immutable.

__cmp__ doesn't really make sense as there is no natural ordering. Your
other points make sense though.

Thanks,

James

« Back to merge proposal