Hi Daniel, Thank you so much for the code review!
I apologize for the late reply. I've updated from your feedback into 3rd version upload: https://mentors.debian.net/package/kvmtool/#upload-3 and also sync to salsa, switching repository to debian/kvmtool: https://salsa.debian.org/debian/kvmtool On 11/26/25 3:06 AM, Daniel Baumann wrote: > Hi Yunseong, > > I had a look at the package: > > * when you create a source tarball yourself (because you use a git > snapshot), please create it from a directory named $package-$version > currently, it's just $package (i.e. kvmtool instead of > kvmtool-0.20251123). > > * when packaging git snapshots, it usually makes sense to be explicit > about which git commit you used and encode that in the upstream > version, in your case that could be 0.20251123+7ad32e5. > > * 'Rules-Requires-Root: no' in control is default, please remove it. > > * 'Priority: optional' can be removed as it's also default now, > however, there are a few tools that can't handle that just yet (like > reprepro), so you might want to keep it for a little while longer. > > * please use wrap-and-sort (part of devscripts) to sort the debian > packaging files. I like 'wrap-and-sort -bast', ymmv. > > * unfortunately, wrap-and-sort doesn't sort the order of fields in > control, currently you have a non-standard order which makes it > unecessarily harder to read. please consider using the default one: > > Source: [...] > Section: [...] > Priority: [...] > Maintainer: [...] > Build-Depends: [...] > Standards-Version: [...] > Homepage: [...] > Vcs-Browser: [...] > Vcs-Git: [...] > > * Same goes for the binary package: > > Package: [...] > Architecture: [...] > Depends: [...] > Description: [...] > > * please don't write "TOOL" in the package short-description in all > uppercase letters. > > * why prefering libsdl1.2-dev over libsl2-dev? If there's no good > reason, I'd use libsl2-dev only. > > * --parallel in rules can be removed, that's default. > > * if you want to micro-optimize, you can use 'install -D' in > override_dh_auto_install to create the target directory and copy > the binary in one 'install'-command. > > however, personally I would rather use debian/kvmtool.install > with 'lkvm usr/bin' in it instead, I prefer declarative use > of debhelper over manual commands. > > * I'd recommend for to use POSIX mode of writing 'test'-statements in > shell, so rather than this: > > if [ -w /dev/kvm && -x /usr/bin/xorriso]; then [...] > > I'd write this: > > if [ -w /dev/kvm ] && [ -x /usr/bin/xorriso ]; then [...] > > * copyright has a non-default order as well, please use: > > Format: [...] > Upstream-Name: [...] > Upstream-Contact: [...] > Source: [...] > > * consider writing copyright more derivative friendly: > > 's/On Debian systems, the complete/The complete/' > > * consider using the standard GPL-2/GPL-2+ license blurbs, see e.g. > the GPL-2 and GPL-3+ ones here (in your case, of course you could > simply replace the '3' in GPL-3+ with a '2'): > > https://tracker.debian.org/media/packages/r/rtrlib/copyright-0.8.0-5 > > * you might want to add a manpage symlink from kvmtool to lkvm. > > * please run lintian on it and fix the remaining warnings > > Regards, > Daniel I've found this tool incredibly useful, and I hope others will find it, too. I wanted to share this tool with debian people at this year's Debconf, as a reminder of its alive. Thank you! Best regards, Yunseong
