Hello Helmut, On 19/11/2014 07:41, Helmut Grohne wrote: > Hi Jean Baptiste, > > On Tue, Nov 18, 2014 at 01:48:04PM +0100, Jean Baptiste Favre wrote: > > Please find attached anew version for the patch. > > Thanks! I still have a question about it. > > > - Provides missing /etc/default/ola from ola postinst script to allow > > olad service control in the same way rdm_test_server is > > I'm not sure why this last point qualifies as serious. I can see that it > is an improvement, but it does not seem like the Debian policy requires > /etc/default/something to be available as a means of control.
/etc/init.d/olad requires it, with a specific content, to start the service. I added it so that users do not have to dig into init script to find what to put into it. I agree this is not a serious issue per se, but I felt like it was an important one, and could be considered as a sort of documentation update. > > - packages pass piuparts tests (I tested .changes file with > > --no-upgrade-test since jessie package fails to install) > > I see that you are learning quickly. Thanks for highlighting the > --no-upgrade-test switch. > > > I also documented verbosely changes in changelog as requested by [1] > > Excellent. > > > Please let me know wheter the work is satisfying or I need to > iterate more. > > Apart from that one change where I don't understand why it is serious, > your patch looks fine to me. Can you explain your reasoning here? Given > the current freeze policy, important issues qualify as well until 5th of > December, so arguing in favour of being important is enough. > > Should there arise a need to reiterate the patch for other reasons than > those below, you can also fix these minor issues if you agree with them: > * s/seriouys/serious/ (changelog) > * "rm -rf /etc/ola" is dangerous and can potentially delete too much. I > suggest to use "rmdir --ignore-fail-on-non-empty" if that makes the > package purge cleanly as well. > * I suggest to remove the summary comments from the new scripts to > reduce the size of your patch. I'll provide a new version this morning with all changes you suggested. Regards, Jean Baptiste
signature.asc
Description: OpenPGP digital signature
