https://bugzilla.novell.com/show_bug.cgi?id=798348 https://bugzilla.novell.com/show_bug.cgi?id=798348#c12 --- Comment #12 from Lukas Ocilka <locilka@suse.com> 2013-01-18 14:02:41 UTC --- The patch is not bad but I have some comments: * First of all, we use GitHub now, where cloning and creating pull requests make it easier for us to incorporate changes from different authors easily - this also makes it easy to comment the change * Some pieces changes or even parts of the diff belong to Service YCP module instead of NetworkService - they are quite generic or at least could be For instance Service::RunInitScript() could be used or even extended ("--no-pager -p Id show" as a param) * Functions like GetServiceId or GetUnitId definitely belong to Service module * Additionally, testsuite is missing -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.