Code review comment for lp:~mbp/bzr/659763-non-ascii-gecos

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

On 4/1/2011 6:04 AM, Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/659763-non-ascii-gecos into lp:bzr/2.3.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #659763 in Bazaar: "bzr smart server can't handle UTF-8 user names, gives UnknownErrorFromSmartServer"
> https://bugs.launchpad.net/bzr/+bug/659763
>
> For more details, see:
> https://code.launchpad.net/~mbp/bzr/659763-non-ascii-gecos/+merge/55862
>
> vila was right that my fix <https://code.launchpad.net/~mbp/bzr/whoami/+merge/55679> also fixed bug 659763. This adds a test for it, cleans up the code a bit, and fixes some (apparently previously untested) broken mutter statements. Because of the last point I think this should go into 2.3 too.

> + def test_auto_user_id(self):
> + """Automatic inference of user name.
> +
> + This is a bit hard to test in an isolated way, because it depends on
> + system functions that go direct to /etc or perhaps somewhere else.
> + But it's reasonable to say that on Unix, with an /etc/mailname, we ought
> + to be able to choose a user name with no configuration.
> + """
> + if sys.platform == 'win32':
> + raise TestSkipped("User name inference not implemented on win32")

This looks more like a missing feature than just a test skipped. (as in,
we don't yet support the feature on windows, or some such).

Off-hand I'm thinking we could use the MAPI support to get an email, and
the username is usually pretty easy to get. Though I don't know it offhand.

> + def test_get_user_name_from_os_smoke(self):
> + """get_user_name_from_pwd may not work but it must not crash."""
> + realname, local_username = config.get_user_name_from_os()
> + self.assertIsInstance(realname, (basestring, type(None)))
> + self.assertIsInstance(local_username, (basestring, type(None)))
> +

^- shouldn't these always be Unicode or None ?

Otherwise looks good to me.

 merge: approve

John
=:->

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

iEYEARECAAYFAk2ViK8ACgkQJdeBCYSNAAP75gCgm5oAGhBjrnksh78XgJ3l1qna
39sAn37HIwKPvSVgXrfgwtQTYQVr7AjR
=GW8X
-----END PGP SIGNATURE-----