On Fri, Jun 1, 2012 at 4:31 AM, Holger Levsen <hol...@layer-acht.org> wrote: > > Hi Dave, > > Thanks, so I've merged the next one... but then I looked at > e6444734909a6074f9f768f0c32ffc56d3a2017e and found several points I disliked, > want improved or plainly reject: >
OK, I think I can return to this productively now. Holger, you asked about background on ht_root. It is a loose reference to 'http root'. Apache defaults to the convention that http:///~<user>/ is served out of the directory ~<user>/ht_root. As Andreas hinted, the analogy to my usage is imperfect. In any case, the issue is moot. I had changed the variable name per your direction before the discussion came up, and I have a one large-variable-renaming-per-customer-per-variable limit. You asked about what "missing templates" I was referring to in an earlier email on packaging. I have had many "analysis template *.tpl does not exist" errors in my piuparts-report output, and no issue summaries in the web output, and chalked that up to cause and effect. I see now that they originate elsewhere. Lately, the web summaries have started showing up. I don't know why. (the latest run has summaries, but no summary counts - odd) The new summaries revealed 6b24129 - detect-well-known-errors - use relative addressing for log links. > > - please keep the package in 1.0 format. 3.0 is more complicated and buys us > nothing as we dont have any patches. I don't understand why 3.0 (native) is more complicated. http://www.debian.org/doc/manuals/developers-reference/pkgs.html#sourcelayout 7bc9779 - Use '1.0' format for the packages The change added a .git lintian warning for me. > > - the README-server.txt (yay for writing it!) says that piuparts stores its > logs in /var/lib/piuparts. Please add a sentence explaining that those logs > are basically the result of piuparts running, thats why we store them in > /var/lib. > > I also wonder whether we should store the master+slave logs in > /var/log/piuparts/... 1515eee - Update server README to explain the logs in /var/lib/piuparts As you point out, the logs don't really belong there. There are a number of topics to be addressed for maturing the packaging which conflict with 'treading lightly' on this branch. I recommend putting them off to a future release. > > - I don't like (/usr)/sbin/slave_run - IMO that should either end up in > /usr/share/piuparts/master/ or rename it to piuparts_slave_run and put it in > /usr/sbin/. I think I prefer the later. > valid. changed in 43ddbf7 - Rename the public links to slave_run/join to piuparts_slave_* and place in /usr/sbin. > > - piuparts-server.postinst: thats totally wrong, like this it would be > executed on any upgrade, removal, purge, etc. You need to interpret $1 and act > accordingly... see > http://www.debian.org/doc/debian-policy/ch-maintainerscripts.html and postinst > scripts of your choice :-) > I read the policy before writing the script, and again now. I understand that what I submitted doesn't match the usual convention for postinst scripts. But, honestly, other than an unchecked reference to ssh-keygen, I don't see a policy violation in it. In any case, as best I can tell from the reference, the script isn't called for removal, purge, etc. 77f08f0 - Add argument support to piuparts-server.postinstall I haven't done a fresh install with this. > - why oh why do you reintroduce piuparts-server.preinst which you removed > (albeit named preinst) in 89926a72e5675218abfbbd6dce50dba0292c43ad? I'm not > impressed :/ > It was code that managed "/etc/piuparts/piuparts.conf", moved to the package that owns that file. b94d703 - Remove obsolete piuparts-server.preinst > - debian/rules: please explain: > > - dh_installman > - $(MAKE) prefix=$(CURDIR)/debian/piuparts/usr > etcdir=$(CURDIR)/debian/piuparts/etc all > + $(MAKE) prefix=$(CURDIR)/debian/tmp/usr > etcdir=$(CURDIR)/debian/tmp/etc all > + dh_install > + dh_installman --sourcedir=debian/piuparts > dh_install supports the creation of multiple packages. It expects the files to be in debian/tmp by default. I've also added a few patches resulting from issues I ran across setting up a new server. b4849f3 - sudoer files with a '.' are invalid. Change file to 'piuparts' 0ce198d - Add server Depends on 'screen' and this one from 'multiple-slaves': c43c9eb - Slave working directory owned by piupartss > > And then finally, I must say I'm disappointed by debian/control, just > introducing a piuparts-server package. I thought we would get piuparts-master > and a piuparts-slave packages instead 8-) > We've looked that this issue and have come to different conclusions on how packaging should be accomplished. You haven't said why you have a strong preference for one strategy over the other. I'd like to understand what your drivers are for wanting the master and slave to be separate. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
