On 8/26/20 10:55 PM, Felix Delattre wrote: > Hi Bas, > > Thanks for the check-in! Please find my answers in between lines: > > On 8/26/20 4:19 PM, Sebastiaan Couwenberg wrote: >> On 8/26/20 5:39 PM, Felix Delattre wrote: >>> Felix Delattre pushed to branch master at Debian GIS Project / >>> libapache2-mod-tile >>> Commits: >>> 24720515 by Felix Delattre at 2020-08-25T20:24:18+00:00 >>> added systemd.service >>> >>> - - - - - >>> 2757a3eb by Felix Delattre at 2020-08-26T15:18:33+00:00 >>> moved helper scripts to a suitable location >>> >>> - - - - - >>> 77d80483 by Felix Delattre at 2020-08-26T15:27:34+00:00 >>> prepared libapache2-mod-tile to work equally with renderd and tirex >>> >>> - - - - - >> >> Commit messages are sentences, they start with a capital and end with a >> period. > > > Is this a suggestion? Or a rule of the team?
It's neither, it's a requirement for good commit messages. >>> ===================================== >>> debian/control >>> ===================================== >>> @@ -1,7 +1,7 @@ >>> Source: libapache2-mod-tile >>> Maintainer: Debian GIS Project <[email protected]> >>> Uploaders: Felix Delattre <[email protected]> >>> -Section: science >>> +Section: web >> >> Don't change the section of the source package. >> >>> Priority: optional >>> Build-Depends: debhelper (>= 10~), >>> dh-apache2, >>> @@ -18,11 +18,12 @@ Homepage: https://wiki.openstreetmap.org/wiki/Mod_tile >>> >>> Package: libapache2-mod-tile >>> Architecture: any >>> -Section: httpd >>> -Depends: libjs-openlayers, >>> - renderd | tirex-master, >>> - ${shlibs:Depends}, >>> +Section: web >>> +Depends: ${shlibs:Depends}, >> >> Especially when you duplicate that same section for a binary package. > > I assumed they should be the same. And I can't really see why this should fit > into "science". > Can you give me some more context please, why do they have to be different? > And why wouldn't it be better to change it to "web"? Why are you changing the source package Section in the first place? It's not making the package function better. GIS provides tools for the geography science, hence this choice for our source packages as documented in the team policy: https://debian-gis-team.pages.debian.net/policy/policy.html#debian-control >>> ===================================== >>> debian/libapache2-mod-tile.apache2 >>> ===================================== >>> @@ -1,3 +1,2 @@ >>> mod src/.libs/mod_tile.so >>> mod debian/tile.load >>> -conf debian/libapache2-mod-tile.conf >> >> Why do you remove the config? > > It is not removed. It is renamed and handled in the `renderd` package. Thus > this prepares `libapache2-mod-tile` for opening to be used by either > `renderd` or `tirex`. That's not clear for your commit messages. >>> ===================================== >>> debian/libapache2-mod-tile.install >>> ===================================== >>> @@ -1,5 +1,3 @@ >>> debian/tile.load etc/apache2/mods-available >>> slippymap.html usr/share/libapache2-mod-tile >>> munin/mod_tile* usr/share/munin/plugins >>> -openstreetmap-tiles-update-expire usr/bin >>> -osmosis-db_replag usr/bin >> >> Why aren't these binaries installed? > > Have you seen them? :) I moved them to via package.examples into > `/usr/share/doc/libapache2-mod-tile/examples` > I don't think they are even close to be responsibly usable in `/usr/bin`. As long as they are not needed for common usage scenarios that's fine. >>> ===================================== >>> debian/libapache2-mod-tile.links deleted >>> ===================================== >>> @@ -1 +0,0 @@ >>> -usr/share/javascript/openlayers usr/share/libapache2-mod-tile/openlayers >> >> Why is this removed? > > The example map should not be created in `libapache2-mod-tile`. It is too > specific and is going to be factored into its own package. Why not? How will installing libapache2-mod-tile have a usable default configuration? >>> ===================================== >>> debian/libapache2-mod-tile.conf → debian/libapache2-renderd.conf >>> ===================================== >>> @@ -1,6 +1,6 @@ >>> -Alias /mod_tile /usr/share/libapache2-mod-tile >>> +Alias /mod_tile /var/lib/mod_tile >>> >>> -<Directory /usr/share/libapache2-mod-tile> >>> +<Directory /var/lib/mod_tile> >>> Options Indexes FollowSymLinks MultiViews >>> AllowOverride None >> >> Why this change? >> >> /usr/share/<package> is the appropriate place for webapps. > > The whole package expects currently things to be in `/var/lib/mod_tile`. This > adjusts it for the sake of consistency. > > I'm happy to move (all of) it to another path. But I don't think > `/usr/share/<package>` is appropriate. As this is not any web app living > there! But rather the generated tiles used for serving. From my (naive) > understanding it should go under `/var` and instead of `/var/lib/<package>` I > would preferably choose `/var/cache/<package>`. What do you think? /usr/share is appropriate for the webapp, i.e. the openlayers bit that's served by apache. /var/cache is indeed appropriate for generated tiles, that's what mapcache uses too for example. >> https://wiki.debian.org/Apache/PackagingFor24 >> >> Please motivate your changes better. > > Please excuse. There are a lot of moving pieces in order to get this to work. > The original files are not in a good shape and many things have to change. > Explaining everything through mail would be tedious. Please look at the > commits and their messages. I usually bundle everything into logical steps. > And I'm always happy to answer questions and react to your comments. When the commits and their messages don't explain the changes sufficiently you get questions like these. Since you mentioned that you have no real experience in packaging we cannot assume that the changes you're making are correct. Kind Regards, Bas -- GPG Key ID: 4096R/6750F10AE88D4AF1 Fingerprint: 8182 DE41 7056 408D 6146 50D1 6750 F10A E88D 4AF1 _______________________________________________ Pkg-grass-devel mailing list [email protected] https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-grass-devel
