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

[Martin Kosek]

On 11/07/2012 04:08 PM, Lynn Root wrote:
> 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
> 

I think this revised patch is pretty much ready. I will just wait if someone
other see some issue. We will need to wait until a post 3.1 release cycle
before we can push this one anyway.

Martin