Merge lp:~mbp/bzr/659763-non-ascii-gecos-2.3 into lp:bzr/2.3

Proposed by Martin Pool
Status: Merged
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/659763-non-ascii-gecos-2.3
Merge into: lp:bzr/2.3
Diff against target: 0 lines
To merge this branch: bzr merge lp:~mbp/bzr/659763-non-ascii-gecos-2.3
Reviewer Review Type Date Requested Status
John A Meinel Pending
Review via email: mp+57098@code.launchpad.net

This proposal supersedes a proposal from 2011-04-01.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote : Posted in a previous version of this proposal

-----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-----

review: Approve
Revision history for this message
Martin Pool (mbp) wrote : Posted in a previous version of this proposal

You're right: the auto_user_id should be marked as a known failure on
windows; and the other one should insist on unicode (which may need a
code fix.)

Martin

Revision history for this message
Andrew Bennetts (spiv) wrote : Posted in a previous version of this proposal

sent to pqm by email

Revision history for this message
Jelmer Vernooij (jelmer) wrote : Posted in a previous version of this proposal

sent to pqm by email

Revision history for this message
Andrew Bennetts (spiv) wrote : Posted in a previous version of this proposal

PQM replied to me: Conflicts during merge: Text conflict in doc/en/release-notes/bzr-2.3.txt

Preview Diff

Empty

Subscribers

People subscribed via source and target branches