[Freeipa-devel] [PATCH] the 'Keytab:' field in "ipa user-show" output is misleading

[Rob Crittenden]

Martin Kosek wrote:
> On Thu, 2011-11-10 at 14:01 +0100, Ondrej Hamada wrote:
>> On 11/10/2011 10:30 AM, Martin Kosek wrote:
>>> On Tue, 2011-11-08 at 20:41 +0100, Ondrej Hamada wrote:
>>>> https://fedorahosted.org/freeipa/ticket/1961
>>>>
>>>> The 'Keytab' filed in output of all 'user-*' commands was changed to
>>>> 'Kerberos keys available'. In order to do this change for 'user-*'
>>>> commands only, the flag 'has_keytab' had to be removed from common
>>>> output parametrs in ipalib/baseldap.py. This change also affected the
>>>> host.py and service.py, where the 'has_keytab' flag was added to their
>>>> local output params. Both host.py and service.py holds the old field
>>>> caption - 'Keytab' - because of compatibility with older clients.
>>>>
>>> Ondra, thanks for the patch. It looks OK, everything behaves as
>>> expected.
>>>
>>> I am still concerned about your patch formatting:
>>> 1) Patch naming does not follow FreeIPA conventions. You can check
>>> others patch file names - mine, Rob's or Alexander's for example. The
>>> patch file name should be freeipa-ohamada-2-<description>.patch. The
>>> patch number should also be in your mail subject - it helps when
>>> searching mails etc.
>>>
>>> 2) Patch title is wrong - you don't need to include [PATCH] in git
>>> commit's title. This then makes it here twice.
>>>
>>> 3) Patch description is insufficient. I miss link to ticket and some
>>> description. You only added it to the mail. When I am traversing FreeIPA
>>> git logs, I must be able to quickly read what this patch does.
>>>
>>> You would have seen all these conventions I wrote you about if you had
>>> read some patches in freeipa-devel or had read some in FreeIPA git log.
>>>
>>> Martin
>>>
>>>
>> Shame on me. Sorry for that.
>>
>> Corrected patch attached.
>>
>
> ACK. Pushed to master.
>
> You can celebrate your first upstream patch :-)
>
> Martin

To make it all official I added Ondrej to Contributors.txt in master.

rob