[Git][debian-gis-team/libapache2-mod-tile][master] 3 commits: added systemd.service

[Felix Delattre]

Hiya,

On 8/27/20 4:26 AM, Sebastiaan Couwenberg wrote:
> 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.

I quite disagree. There are good arguments for other styles of commit messages. And they seem to be used in debian
But I will follow your desire. And I suggest we add it to the Debian GIS policy.
 
> 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.

It is not?

Commit message: "prepared libapache2-mod-tile to work equally with renderd and tirex"cd 

And the snipped from the commit:

> rename from debian/libapache2-mod-tile.conf
> rename to debian/libapache2-renderd.conf


>>>> =====================================
>>>> 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?

First, it is not working. In order to make it working well, more things need to be added. I don't want to have this in the main package.

> How will installing libapache2-mod-tile have a usable default configuration?

The most usable default configuration for libapach2-mod-tile is none. It only works with renderd or tirex, and either of those packages is going to provide the config euch of it needs.

> 
>>>> =====================================
>>>> 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.

ok, I am going to switch to /var/cache then.
 
>>> 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.

I'm fine with the questions! But I think I'm also doing pretty clear commits.

> Since you mentioned that you have no real experience in packaging we
> cannot assume that the changes you're making are correct.

Thanks for checking carfully and giving me feedback!

Cheers,
Felix