Code review comment for lp:~ericmoritz/bzr/488519-walkdirs-encoding-bug

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Bennetts wrote:
> You have been requested to review the proposed merge of lp:~ericmoritz/bzr/488519-walkdirs-encoding-bug into lp:bzr.
>
> A fix for bug #488519
>
>

This is the 'walkdirs' code and not the '_walkdirs_utf8' code, the
latter being the more performance critical code. And he is correct that
we are already casting everything to Unicode, except for what Python
cannot cast based on fs_encoding. And the only way we can determine that
is by checking the type of the string. As such, I think his change is
reasonable.

I will note that:

a) I prefer "if type(filename) is unicode" versus the isinstance check,
but it shouldn't be a huge difference. (isinstance has to check the
inheritance heirarchy, but I would assume that being exact 99.9% of the
time means it would still be fast.)

b) I expect that if we get a string, then the decode will fail. That is
sort of the crux of the problem (python listdir() failed to decode it,
so it just returns a string, I don't know why we would expect to find
differently.)

c) It would be better to put ByteStreamedNamedFilesystem into
bzrlib/tests/features.py and call it "byte_stream_named_filesystem". But
maybe not since the other ones like UTF8Filesystem are still in this
location.

The rest seems ok as long as the other reviewers are happy.

 review: approve
John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwOn90ACgkQJdeBCYSNAAObeACgqq60DJ3Jy3T3MnZ4ZSERL4gK
X4YAoMH9WydE+dCoYL1c4MEmEQzie42y
=Ctuq
-----END PGP SIGNATURE-----