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

[Martin Basti]

On 22.04.2016 05:02, Rob Crittenden wrote:
> 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.
+1
>>>
>> 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
>
Patch do exactly the same as was agreed on devel meeting. I don't see 
reason why we shouldn't fix oneliner.

Pushed to master: a023dcbc5c2e60d29938588845905b62986c3a93