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

Ondrej Hamada ohamada at redhat.com
Thu Nov 10 13:01:06 UTC 2011


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.

-- 
Regards,

Ondrej Hamada
FreeIPA team
jabber: ohama at jabbim.cz
IRC: ohamada

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ohamada-2-2-Misleading-Keytab-field.patch
Type: text/x-patch
Size: 5152 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111110/38889943/attachment.bin>


More information about the Freeipa-devel mailing list