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`! >
signature.asc
Description: OpenPGP digital signature