Hi, On Mon, 19 Dec 2016, Joseph Herlant wrote: > I migrated the package from dpatch to quilt and cut the tag. > I don't know if you prefer to work directly from the repo or from > mentors, so I uploaded it to mentors too: > https://mentors.debian.net/package/asciidoc > Any feedback appreciated.
I reviewed your git commits and I found that I often have changes unrelated to what you describe in your git commit. This makes reviewing really painful. If you have to reformat some files (to align columns, to better wrap, etc.) please do it in a separate commit. Here when I review 7a7b6fa57981a1ff081c5ec0579ce65191162c82 I don't want to see so many changes on debian/asciidoc.install. I want only the vim line dropped and see it added in the new package. Now reviewing the split (c93478fd9be3ad2f9841df973eb7b09d312bd699) I did not check in details the files repartition but I noticed this: * epubcheck is in suggests everywhere but I guess it's not usefull for all backends, it probably makes sense only in asciidoc-base. * source-highlight is also mentionned everywhere but since ascidoc-base is required, it's enough to be on that package. * When you split packages in a fine-grained level, you should move the optional "Recommends" into a required "Depends". asciidoc-fop without fop doesn't make sense, thus fop and docbook-utils should be Depends and not Recommends. Same with "dblatex" and docbook-utils in asciidoc-dblatex. Looking at your clean target in debian/rules, you can replace it entirely with a "debian/clean" file (that said I wonder why some of those "rm" are there, what creates debian/asciidoc.1.xml, etc?). Last detail, lintian reports this informational tag (on all binary packages): I: asciidoc-fop: debian-news-entry-uses-asterisk N: N: The latest entry in NEWS.Debian appears to use asterisks to present N: changes in a bulleted list, similar to the normal changelog syntax. N: The Debian Developer's Reference recommends using regular paragraphs N: in NEWS.Debian rather than a bulleted list. N: N: Refer to Debian Developer's Reference section 6.3.4 (Supplementing N: changelogs with NEWS.Debian files) for details. N: N: Severity: wishlist, Certainty: possible N: N: Check: changelog-file, Type: binary N: Can you clean up those little details quickly? How long have you been using your updated packages already? Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: http://www.freexian.com/services/debian-lts.html Learn to master Debian: http://debian-handbook.info/get/
