Code review comment for lp:~rackspace-titan/nova/osapi-serialization

Revision history for this message
Ed Leafe (ed-leafe) wrote :

> So the best thing to do in this case would be to use the built-in interfaces.

No, the best thing to do would be to change:
    if type(data) is list:
to:
    if isinstance(data, list):

> First assume the 'data' variable is a dict and attempt to use it as such. When
> that doesn't work out, assume it is a list. When that doesn't work we end up
> just adding it as an xml text node. I'm going to stand by my point that I
> don't want to touch the code you are referring to since it is serving its
> purpose without issue. If someone wants to refactor the actual xml-specific
> serializer, I welcome it :)

Since the code is suboptimal but not incorrect, it is not imperative that it be changed, which is why I suggested simply adding a #TODO comment. I'm not sure why adding a comment about a better way to write the code counts as "touching" the code; that's what I thought the whole point of #TODOs were.

review: Approve

« Back to merge proposal