[MERGE] Fix bencoding of bools, provide a bdecode_tuple function.
John Arbash Meinel
john at arbash-meinel.com
Tue Jan 27 15:35:36 GMT 2009
John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
So this is ok, though there are a few bits to consider
1) Copyright. Not sure exactly what to do here, but the code was written
by Petru Paler, and this is a fairly large reorganization. Reading over
it, I almost wonder if it is fully GPL compatible just because of this
clause:
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.
Though I think that one is ok, and it is the one about advertising using
the code that the GPL doesn't like about BSD 4-clause.
Anyway, we don't have to do anything here, but I at least wondered about
it.
2) Having Bools encoded as "i1" is good, though you don't get your bools
back out again, which is a bit of a shame.
3) I was a bit surprised that you used an "if self.yield_tuples" inside
the "decode_list" function, rather than having a separate
"decode_list_as_tuple()" function, which got inserted into the
dictionary when "yield_tuples=True". I don't think it matters, just was
more what I was expecting.
4) Would we want to use __slots__ on this object, since it seems we only
have a couple attributes, but we access them often.
5) As for other names "bdecode" is fairly obvious, but "bdecode_tuple"
sounds like it is decoding a single tuple. Perhaps "bdecode_as_tuples"?
Or maybe a different formulation entirely "bdecode(X, as_tuples=True)".
I think your patch is ok, and it is fine if you want to merge it as is.
I just wanted to offers some other things that I thought of, which would
be a different way of accomplishing this (might be cleaner/clearer,
might not).
For details, see:
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C20090107022415.GA2394%40steerpike.home.puzzling.org%3E
Project: Bazaar
More information about the bazaar
mailing list