Hi,

thanks for the detailed review, that was very helpful!

On 2/9/19 6:50 PM, Sean Whitton wrote:
>> Changes since beta2:
>>   * New upstream release
>>   * Remove swaylock and swayidle: they are now separate packages
>>   * Clean up d/sway.install
> 
> "Cleaning up" suggests only deleting lines, but you also added some
> wildcards, so "tidied up" would be better.  This is an extremely minor
> point :)

  * Tidied up d/sway.install: now that swaylock and swayidle
    are seperate source packages, its easier to use wildcards

>>   * Specify versioned build-dependencies and add libelogind
>>     and libsystemd build-dep
>>   * Remove references to swaygrab
> 
> You need to mention the new wayland-protocols build-dep, too.
> 
> Normally you would also specify /why/ the build-deps now need to be
> versioned (someone should not need to guess why the scdoc build-dep
> needs to be 1.8.1, for example).  Since this is a NEW package, it
> doesn't matter, so I'm telling you about this for the future.
> 
> It would also be worth mentioning why swaygrab is no longer referenced.
> Again, though, it doesn't matter because this package is NEW.

  * Specify versioned build-dependencies as required by upstream.
    Thus explicitly listing wayland-protocols and libsystemd as
    build-dep with versions and adding versions to scdoc build-dep.
  * Adding libelogind build-dep
  * Bump version of libwlroots-dev build-dep
  * Remove references to swaygrab (it was removed from the upstream
    package; see grim(1) for an alternative)

>>   * Update Recommends and Suggests
> 
> Similarly, more detail would be useful, though it doesn't matter much
> since the package is NEW.

  * Update Recommends and Suggests: one of the sway wallpapers is
    configured as background in the shipped config file, thus the
    package containing the wallpapers is recommended. swayidle and
    swaylock are also configured in the shipped config file, but
    commented, thus they are only suggested
> 
>>   * Make versioned run dependency on libwlroots0 explicit
> 
> Please explain (either in the changelog or with a comment in d/control)
> why this is needed -- it's unusual not to be able to rely on
> dpkg-shlibdeps doing this for you.  This does matter, even if the
> package is NEW.

I have to admit that i don't know ;) When trying to test sway i realized
that i only had libwlroots0 in an older version but dpkg didn't warn me
about that. `apt show sway` didn't show the version for libwlroots0
either in the Depends. But with listing the version explicitly the
package also has a dependency on the correct version.

>> There are new lintian warnings:
>>
>> I: sway: file-references-package-build-path usr/bin/sway
>> [...]
>> I think this refers to the occurrence of strings like:
>> ../sway/commands/exec.c
>> in the binaries. This is a relative path and the lintian warning is
>> about reproducability, so i think this is a false negative?
> 
> This doesn't need to block an upload to experimental, so I'll leave it
> to your judgement.  Thanks for investigating.
Oke, lets ignore this for now, i'll try to figure out if its a problem
with sway or with the lintian warning


> I haven't tried installing and running the package yet, btw, but I guess
> that you have done this.  I'll give it a try before the upload.
yes, i'm running it ;)

cheers,
Birger

> 
> After you've responded to the above, please remember to `dch -r`!
> 

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to