On Thu, Apr 04, 2019 at 10:20:47AM +0000, Dmitry Bogatov wrote: > * Please, do not use ${runit:Conflicts}. As suggested by Lintian, it is > very strong relations. Use ${runit:Breaks} only. The very > introduction of ${runit:Conflicts} was my mistake. It will make > Lintian override unneeded.
Will do. > * Please, consider merging bin:socklog-run into bin:socklog. Option > `since' for dh_runit will be useful for it. Ah that's what you meant, sorry I misunderstood. I will try to merge them. > * I know, it is pain, but there should be init.d script. You may want to > take a look at bcron=0.11-8. Sure, no worries. How about systemd service files? It makes little sense to run socklog along with systemd I think, but for the principle it may be required to profile service files. What do you think? > * Please, add description to 0002-import-patch-tryto. It is unclear, > what issue this patch resolves. This one comes from the previous packaging scripts. I'll do some digging. > * In patch 0003-remove-chkshgrp you remove test, that fails on CI. Does > it also fails in sbuild? If not, probably it should only be disabled in > Gitlab CI? It only fails in CI. I'll try to have it disabled in CI only, that would be much better indeed. > * It is matter of taste, but are you aware, that you can > > Build-Depends: debhelper-compat (= 12) > > and remove `debian/compat'? I didn't know, thanks :) > * Dep-5 would be nice. Will do. > * What is the purpose of 'log' user, you create with dh_sysuser? You > know, that bin:runit provides user `runit-log' since -20, don't you? I overlooked that, thanks. > * All services run as same user, 'nobody'. It is unfortunate, since nobody is > quite popular owner for NFS files. > > I believe there should be separate sysuser for socklog-* services. > Ideally, separate sysuser for /every/ from socklog-* service, but I do > not know, whehter it is possible. Yeah good point. I tend to think that a single user for all socklog-* services would be enough, but if you prefer I can add one user per service. > * I believe, README file is useless -- it contains copyright, authorship > and homepage information only, which is already present in Debian > package files. Alright, I'll remove it. Thanks for the review! Cheers, -- Mathieu Mirmont <m...@parad0x.org>
signature.asc
Description: PGP signature