[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