On Thursday, July 12 2018, Lars Kruse wrote: > Hello Sergio,
Hey Lars, > thank you for reviewing my packaging attempt! No problem. Thank you for packaging for Debian :-). > Am Wed, 11 Jul 2018 23:21:05 -0400 > schrieb Sergio Durigan Junior <sergi...@debian.org>: > >> [..] >> 1) On d/copyright, you don't list the files under the "debian/" >> directory. These should be listed, and you should be the author. I >> recommend choosing the same license as upstream, just to make things >> simpler. >> >> 2) On d/control, Standards-Version should now be 4.1.5. >> >> 3) Better safe than sorry: on d/control, the Vcs-* fields should point >> to your salsa.d.o repo. > > Done (1-3). > > >> 4) On d/rules, you can remove the "override_dh_auto_install" target if >> you're not using it. > > Done. > I am a bit embarrassed, that I overlooked that cruft ... No need to be embarassed. That's why we do reviews :-). Hm, it seems you removed an important line, the one that was exporting the HOME variable. It's needed because the Makefile uses it to determine the install location. Also, some things I forgot to mention in my first review: 1) Since this is the first release of the package, a d/changelog with an entry like: sharness (1.0.0-1) unstable; urgency=medium * Initial release. (Closes: #902730). -- Lars Kruse <de...@sumpfralle.de> Fri, 13 Jul 2018 00:47:01 +0200 would have been enough. But this is just to save you time; I definitely don't complain about extensive changelogs! 2) A word about git tags. I noticed you only have the "debian/1.0.0-1" tag, but no "upstream/1.0.0" tag in your git repo. git-buildpackage should have created that for you; it's worth checking to see if you didn't forget to push. Also, we usually don't create the "debian/xyz" tags until the package has been uploaded and accepted into the archives. This is because the "debian/xyz" tag is meant to indicate when a successful release of the package was made. > My recent upload contains another change besides the ones above: for now I > removed the helper script "aggregate-results.sh". > I started a discussion with upstream in order to decide whether that file > needs > to be included at all (https://github.com/chriscool/sharness/issues/78). Hm, alright. The "aggregate-results.sh" may be useful to some users; it provides a way to display the results in a nicer way, right? I understand the concerns you raised in the ticket, and don't have an immediate answer. Maybe you could install it under /usr/share/doc/sharness/contrib/, since it's not an example script, but doesn't seem *that* important to justify polluting the PATH. What do you think? >> Otherwise, the package looks good to me. Let me know when you address >> these issues and I'll be happy to upload it. > > Thank you for your kind offer! > I just uploaded my updated source package: > https://mentors.debian.net/debian/pool/main/s/sharness/sharness_1.0.0-1.dsc Thanks. I'll wait until you fix the issue with d/rules (readding the HOME variable), and then upload the package. Cheers, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/
signature.asc
Description: PGP signature