[Freeipa-devel] [PATCH] 957 ipa-client-install: fix typo in nslcd service name

[Rob Crittenden]

Lukas Slebodnik wrote:
> On (21/04/16 16:42), Rob Crittenden wrote:
>> Lukas Slebodnik wrote:
>>> On (21/04/16 19:25), Petr Vobornik wrote:
>>>> related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>>>> --
>>>> Petr Vobornik
>>>
>>>>  From b9b8716ec9ba5a5cdbed1f6cdedf7cff8878f08f Mon Sep 17 00:00:00 2001
>>>> From: Petr Vobornik <pvoborni at redhat.com>
>>>> Date: Thu, 21 Apr 2016 19:23:31 +0200
>>>> Subject: [PATCH] ipa-client-install: fix typo in nslcd service name
>>>>
>>>> related but does not implement https://fedorahosted.org/freeipa/ticket/5806
>>>> ---
>>>> client/ipa-client-install | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/client/ipa-client-install b/client/ipa-client-install
>>>> index c38843f85639a9118cd1a471709992690643d79a..0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e 100755
>>>> --- a/client/ipa-client-install
>>>> +++ b/client/ipa-client-install
>>>> @@ -2938,7 +2938,7 @@ def install(options, env, fstore, statestore):
>>>>                   nscd.service_name)
>>>>
>>>>       nslcd = services.knownservices.nslcd
>>>> -    if nscd.is_installed():
>>>> +    if nslcd.is_installed():
>>>>           save_state(nslcd)
>>>
>>> I thought that milestone "Future Releases" has lower priority
>>> then "FreeIPA 4.4 Backlog"
>>>
>>> Therefore I would prefer to close ticket #5806 and implement following one
>>> https://fedorahosted.org/freeipa/ticket/5557#comment:2
>>
>> I don't understand what you are suggesting. Tickets aren't swapped like this
>> and certainly non-related bugs aren't closed for another.
>>
>> This patch just fixes an obvious one-liner as agreed upon in triage. The rest
>> will be potentially addressed later.
>>
>> ACK on the patch.
>>
> I cannot see a reason to fix oneliner.
> This code is not tested and should be removed (#5557)

I fail to see the controversy here. These are two completely and 
absolutely unrelated bugs.

> Or someone should write integration test.
>
> I'm sorry but conditional-NACK (missing integration test)

I've been out of the project a while, so perhaps my knowledge is stale, 
but AFAIU upstream QE writes the integration tests, not the developers. 
This was at least true when I was a daily contributor.

At least at one time QE did testing using --no-sssd. Whether that is 
still the case I don't know but I do remember fixing bugs around it. And 
this particular bug might not be immediately apparent unless a specific 
configuration was present. I noticed the bug while reading code, not in 
production.

rob