[Freeipa-devel] [PATCH] 088 Check IPA configuration in install tools

[Jan Cholasta]

On 18.7.2011 15:00, Martin Kosek wrote:
> On Mon, 2011-07-18 at 14:35 +0200, Jan Cholasta wrote:
>> On 18.7.2011 09:41, Martin Kosek wrote:
>>> On Fri, 2011-07-15 at 10:14 -0400, Rob Crittenden wrote:
>>>> Martin Kosek wrote:
>>>>> On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote:
>>>>>> Martin Kosek wrote:
>>>>>>> Install tools may fail with unexpected error when IPA server is not
>>>>>>> installed on a system. Improve user experience by implementing
>>>>>>> a check to affected tools.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/1327
>>>>>>> https://fedorahosted.org/freeipa/ticket/1347
>>>>>>
>>>>>> Can you add a docstring to the check_server_configuration() function?
>>>>>>
>>>>>> Looking in each utility it isn't necessarily obvious what this does but
>>>>>> my meager attempts at renaming it all failed. I considered
>>>>>> is_server_installed() but that implies it would return True/False. Then
>>>>>> I considered require_server_configured() but that didn't seem to fit
>>>>>> either. We have lots of other check_* so I guess it is fine, but some
>>>>>> docs on where/why it is used would be nice.
>>>>>>
>>>>>> rob
>>>>>
>>>>> I see you undertake the same function naming dilemma as I do. I improved
>>>>> documentation for the function, it should help.
>>>>>
>>>>> Martin
>>>>
>>>> ACK
>>>
>>> Merged to current master. Pushed to master, ipa-2-0.
>>>
>>> Martin
>>>
>>
>> I've just tried to build current master and got this:
>>
>> ./make-lint
>> install/tools/ipa-replica-prepare:68: [E0602, parse_options] Undefined
>> variable 'config'
>>
>> Does anyone run make-lint before submitting a patch or during review at
>> all? :(
>>
>> Honza
>>
>
> We don't - so that you can rant on the list :-) Of course we do, but
> this one slipped in. Thanks for catching this.
>
> Fixed and pushed under the one-liner rule (patch attached).
>
> Martin

That's a relief, I got frightened for a moment :-)

Honza

-- 
Jan Cholasta