[merge] bzr whoami

[John Arbash Meinel]

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

Robey Pointer wrote:
> 
> On 1 Jul 2006, at 19:43, John Arbash Meinel wrote:
> 
>> ^-- Three comments on the copyright statement.
> 
> Rather than responding line by line, I'll just say that all of these
> comments are referring to the original code.  I corrected them and
> submitted them.  But now to avoid 20 more rounds of this, I'm going to
> ask for an up/down vote as-is.
> 

I'm sorry if it feels dragged out. One of the good/bad things about code
review is that it reminds us what is going on in that area, and gives us
 a chance to clean things up.

I've only been having you do more work because I like what you've done,
and want it integrated. So if we can spend a little bit of time cleaning
up what was done in the past, it just makes it nicer overall.

> 
>> You only need to set the 'encoding' parameter if you want to force the
>> sys.stdout() encoding.
> 
> That's actually the point.  The tests verify that 'whoami' works for at
> least those two encodings, using an identity that can't be expressed in
> ascii.
> 

Sure, I see your point.

> 
>> So one thing we need to decide is if 'whoami' should fail if it cannot
>> print your name properly in the current encoding. I'm a little torn. On
>> one hand I would like to be strict, because a shell front end might use
>> whomai.
>> On the other, it might confuse users if they get an encoding error while
>> trying to just check if they set their username correctly.
> 
> I don't have a strong opinion on this either.
> 

For now, we'll just leave it as is, with 'replace'.

> 
>> v- in general, I prefer 'run_bzr' to runbzr. I personally think runbzr
>> should go away. (It only exists on ExternalBase, not TestCaseInTempDir).
> 
> I just missed that one when migrating the rest.  I do think that it's
> utterly confusing which one of the various run*bzr* methods is meant to
> be called.  Why not mark the other ones as deprecated?  I picked
> 'run_bzr' pretty much by personal choice since I couldn't remember which
> one was preferred from the last time the 'whoami' patch failed to land.
> 

I agree, which is why I think run_bzr should die. :)
But it is used in enough code that doing a cleanup isn't very trivial.
And they do have slightly different semantics (runbzr() splits on
whitespace).

> 
>> Something doesn't seem right here. I really don't like the look of
>> WorkingTree.open_containing('.')[0].branch.get_config()
>>
>> Also, because whoami is a branch config, it should still work even
>> without a working tree. So I would rather see:
> 
> Done.
> 

Thanks.

> 
>> I think we may want to validate that the supplied value really is a:
>>
>> "Full Name <email at address.com>"
>>
>> Rather than just letting them do:
>>
>> bzr whoami george
>>
>> We do accept short names (I don't think we explicitly require email
>> addresses anywhere). But we would probably at least want to warn the
>> user that it is recommended to use a full email address, etc.
> 
> I think that's a good idea.  I would want to just reuse the method that
> does this extraction in config, so it should be a separate patch.
> 
> robey


Sure. I realize you have a branch for this, but is there any reason you
didn't post a patch?

Anyway, I'll give you a +1, I'm attaching the diff so someone else can
review it. And if we don't here anything, ping me in a few days and I'll
merge it.

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

iD8DBQFEqSXXJdeBCYSNAAMRAiQyAJ4gqKV+FWdGeCkatzydwyJiKNIGngCcDLwx
9D9ROqtq7pqGiRDomCE56X0=
=U/18
-----END PGP SIGNATURE-----
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: whoami-can-set-value.diff
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20060703/f369925d/attachment.diff