[Freeipa-devel] [PATCH] Switch %r specifiers to %s in Public errors

[Lynn Root]

Third time is a charm?

Lynn Root
Associate Software Engineer
Red Hat

----- Original Message -----
From: "Jan Cholasta" <jcholast at redhat.com>
To: "Lynn Root" <lroot at redhat.com>
Cc: freeipa-devel at redhat.com
Sent: Monday, November 5, 2012 10:25:32 AM
Subject: Re: [Freeipa-devel] [PATCH] Switch %r specifiers to %s in Public errors

On 5.11.2012 09:43, Lynn Root wrote:
> Here's try #2! Adjusted patch attached.  Let me know if there's anything else I've missed.
>
> Switched %r specifiers to '%s' in Public errors, and adjusted tests to expect no preceding 'u'.
>
> Tickets: https://fedorahosted.org/freeipa/ticket/3121 & https://fedorahosted.org/freeipa/ticket/2588
>
> Lynn Root
> Associate Software Engineer
> Red Hat
>
> ----- Original Message -----
> From: "Martin Kosek" <mkosek at redhat.com>
> To: "Jan Cholasta" <jcholast at redhat.com>
> Cc: "Lynn Root" <lroot at redhat.com>, freeipa-devel at redhat.com
> Sent: Tuesday, October 30, 2012 9:08:33 AM
> Subject: Re: [Freeipa-devel] [PATCH] Switch %r specifiers to %s in Public errors
>
> On 10/30/2012 09:04 AM, Jan Cholasta wrote:
>> Hi,
>>
>> On 29.10.2012 19:54, Lynn Root wrote:
>>> Hi all!
>>>
>>> This switch drops the preceding 'u' from strings in public error messages.
>>>
>>> Ticket: https://fedorahosted.org/freeipa/ticket/3121
>>>
>>> This patch also addresses the unfriendly 'u' from re-raising errors from the
>>> external call to netaddr.IPAddress by passing a bytestring to the function.
>>>
>>> Ticket: https://fedorahosted.org/freeipa/ticket/2588
>>>
>>>
>>> My first patch (and freeipa dev list email) ever! Let me know where there's
>>> room to improve.
>>>
>>> Lynn Root
>>> Associate Software Engineer
>>> Red Hat
>>>
>>
>> I think it would be nice if you kept the quotes around the values, as that is
>> probably the reason "%r" was used in the first place - i.e. use "'%s'" instead
>> of plain "%s".
>
> +1
>
> With current patch, I assume that a lot of unit tests will fail as they check
> exact error message wording. I'd recommend running the whole test suite with
> your second patch revision. There is a short walkthrough how to set it up:
>
> http://freeipa.org/page/Testing
>
> Martin
>

You missed a few:

$ git grep -En '%(\(.*?\))?r'

Honza

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Switch-r-specifiers-to-s-in-Public-errors-for-user-f.patch
Type: text/x-patch
Size: 44687 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121107/ffc78ce9/attachment.bin>