Hi Apologies for the confusion. I accidentally sent the previous email using the wrong identity due to a mail client misconfiguration.
This is Yunseong Kim. Please disregard the previous sender name! Best regards, Yunseong Kim On 3/11/26 10:03 PM, Paran Lee wrote: > 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 >
