Re: [LTP] [PATCH] cron_tests.sh: fix a bug of syslog daemon is disabled

[Peng H. <pe...@cn...>]

On 07/03/2013 07:18 PM, ch...@su... wrote:
>>> We have similar code to restart cron in cron/cron02 and to restart nscd
>>> in sssd/sssd-libs.sh.
>>>
>>> What about we create a short library with a common code to restart
>>> system daemons? So that any new testcase would just need to source the
>>
>> A short library is in cmdlib.sh.
>> stop_daemon, status_daemon and restart_daemon is used in the patch.
> 
> Then I do not understand why the patch adds systemctl detection
> code to the cron_tests.sh. It is allready present in the cmdlib.sh
> 
>> The syslog daemon may be syslog, rsyslog, syslog-ng etc, 
>> so I make the patch which is similar to syslog-lib.sh.
> 
> Which is why I object to the patch, it copies code from library to the
> testcase, which is wrong in terms of longterm maintenance.
> 
> Ideal solution would be to move the syslog detection to the cmdlib.sh
> and make a canonical name for syslog so that if you request to restart
> the daemon in test with:
> 
> daemon_restart syslog
> 
> Everything is detected automatically, the syslog binary, systemd vs
> system V init, etc. And everythig is kept in one file, no code
> duplications.
> 

I see, thanks.
I will send v2 patch.

-- 
Best Regards,
Peng