Re: Bug#832985: RFS: svgsalamander/1.0.0+dfsg1-1
Hi Felix, Thanks for your changes. On 08/06/2016 04:35 PM, Felix Natter wrote: > Sebastiaan Couwenberg writes: >> Consider adding the --parallel option to dh in debian/rules to enable >> the use of parallel builds with DEB_BUILD_OPTIONS="parallel=". > > done. (although compilation takes less than a few seconds). The benefit of parallel builds for small packages is limited, adding the --parallel option is mostly a best practice because without it debhelper won't enable parallel support even when it's beneficial. I maintain a couple of big packages for which you don't won't non-parallel builds as those take several hours, instead of under an hour with DEB_BUILD_OPTIONS="parallel=3". debhelper compat level 10 defaults to --parallel for all buildsystems that support parallel building, which is a nice improvement. But compat level 10 is not well supported in stable yet. >> The watch file can also be improved to handle common issues [0], like >> the attached version for example. > > I added your watch file, thank you. > So that I understand this: > - version=3 is preferred > - make archive type variable > - make dversionmangle more general (backports etc.) > - make uversionmangle more general: > +uversionmangle=s/(\d)[_\.\-\+]?((RC|rc|pre|dev|beta|alpha)\d*)$/$1~$2/;s/RC/rc/,\ > --> is this best practice for github tarballs? Otherwise I think this is > difficult because every upstream project has different terminology. > > --> Maybe the uscan man page should be extended regarding this? > (it contains a github example hard-coded for tar.gz) Because uscan in jessie doesn't support version=4 yet, I prefer version=3 watch files until stretch is stable. The downgrade for svgSalamander is appreciated. The uversionmangle is a best practice in general, and is documented on the wiki [0]. Because the gbp import-orig complains about uppercase RC that is additionally translated to lowercase. Because the version captured in debian/watch included non-digets (\d+\S+) you need to handle pre-releases with the uversionmangle rule, otherwise version 1.0.0-rc1 will not precede version 1.0.0. [0] https://wiki.debian.org/debian/watch#Common_mistakes >> Also consider adding upstream metadata [1]. > > I added this. I've committed a few improvements to the upstream metadata before sponsoring the upload. Most importantly fixing the Repository URL to include the .git suffix required for `git clone`. I've also added the Repository-Browse field (without the .git suffix). I've also added a gbp.conf file to use pristine-tar by default, to not require the --(git-)pristine-tar options for the git-buildpackage commands. Kind Regards, Bas -- GPG Key ID: 4096R/6750F10AE88D4AF1 Fingerprint: 8182 DE41 7056 408D 6146 50D1 6750 F10A E88D 4AF1
Re: Bug#832985: RFS: svgsalamander/1.0.0+dfsg1-1
Sebastiaan Couwenberg writes: > Hi Felix, hello Bas, thank you very much for the detailed review. > I've had a look at your package and some comments follow. In general the > package looks good, but there is room for improvement. > > Please consider bumping the debhelper compatibility to 9. done + cme fix dpkg-control. > Also change the LGPL-2.0 shortname to LGPL-2+ to better reflects the "or > (at your option) any later version" clause. done. > The Forwarded header in > 0004-Use-system-awt-gradient-instead-of-the-embedded-batik.patch & > 0006-modify-broken-upstream-pom.patch can also be improved. For the > former "not-needed" is more appropriate than "no" with note, not-needed > is probably also appropriate for the latter. done. Upstream knows about the broken POM (0006), so I put in not-needed there. > The README.source should be updated to reflect the change to GitHub > tarballs. done. I put in both "signatures", since some stuff is from Nicolas. [...] See the get-orig-source target for debian/rules to fetch a clean tarball. -- Felix Natter Sat, 6 Aug 2016 15:41:00 +0200 -- Nicolas Dandrimont , Sun, 6 Mar 2011 15:07:47 +0100 > Consider adding the --parallel option to dh in debian/rules to enable > the use of parallel builds with DEB_BUILD_OPTIONS="parallel=". done. (although compilation takes less than a few seconds). > The watch file can also be improved to handle common issues [0], like > the attached version for example. I added your watch file, thank you. So that I understand this: - version=3 is preferred - make archive type variable - make dversionmangle more general (backports etc.) - make uversionmangle more general: +uversionmangle=s/(\d)[_\.\-\+]?((RC|rc|pre|dev|beta|alpha)\d*)$/$1~$2/;s/RC/rc/,\ --> is this best practice for github tarballs? Otherwise I think this is difficult because every upstream project has different terminology. --> Maybe the uscan man page should be extended regarding this? (it contains a github example hard-coded for tar.gz) > Also consider adding upstream metadata [1]. I added this. New Changelog: svgsalamander (1.0.0+dfsg1-1) unstable; urgency=medium * New upstream version, now on github * Update README.source (source pulled from github release) * Add watch file (from Sebastiaan Couwenberg ) * Use Files-Excluded: instead of repack script * Do not call netbeans ant targets by setting mkdist.disabled * Fix the pom to be installed by maven-repo-helper (version=1.0.0, no deps) * Update standards-version to 3.9.8 (no changes) * Fix license short names (BSD->BSD-3-clause, LGPL-2.0->LGPL-2+) * Fix lintians * Add DEP3 patch headers * Add upstream metadata * Bump debhelper compat to 9 -- Felix Natter Sat, 06 Aug 2016 16:16:37 +0200 Many Thanks and Best Regards, -- Felix Natter
