[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#822722: RFS: tldp/0.7.7 [ITP #822181]



control: owner -1 !
control: tags -1 moreinfo

Hi Martin, lets pick up this package!

First a general note: I'm truly impressed about your work and your learning curve!

Don't be sad for my review, the package works, so I guess the review is about minor
issues and nitpicks!



>control: reassign -1 sponsorship-requests


thanks Mattia for being subscribed to "unknown" packages :)


>Well, I'm on the 'debian-mentors@lists.debian.org' mailing list.  Is 
>that what you mean by mentors.d.n?


http://mentors.debian.net

>I am happy to make changes to my package to try to make it conform.  
>In order to do so, I simply need more guidance.  For me, it would 
>help to see an example of a package that provides a debian/ 
>packaging directory, but is not hosted on alioth.


usually you can do this way:

maintain an upstream "master" branch without debian directory

maintain a debian branch with the debian packaging and the upstream one

git merge master when a new release is out in debian branch, and dpkg-buildpackage it
(using the upstream tarball to be sure there aren't unwanted changes).

or gbp buildpackage -S -sa --git-debian-branch=debian

this will avoid having debian stuff in the upstream tarball, and letting you develop debian/specific
fixes without having to release a new tarball

>Well, I was able to build the package (straight from checkout) using 
>'debuild', 'debuild-pbuilder' and 'gbp buildpackage'.
>
>Also, I did not know about the clean 'sid' chroot, despite having 
>plowed through the mountain of documentation.  Thank you.


it seems to be building on a clean sid chroot.

>OK.  Probably if I use a 'sid' instance, that will work.


yep
>(I have not running lintian myself, rather I'm letting it get called 
>by the build scripts.)


wonderful


and now the rest of the story
(I'm not sure, probably in the last release the native/quilt stuff is fixed)
1) debian/control:
Depends: ${misc:Depends},
${python3:Depends},
python3,
python3-networkx


I would drop python3 and python3-networkx, because they should be picked up at build time
by the python3:Depends variable (being listed in install_requires).

can you please remove them and try to see in the built .deb file if they are picked at runtime correctly?
(just open the deb file and look debian/CONTROL file or whatever is called nowadays)

- please consider adding the Python2 or a tldp binary package

2) debian/rules
-b man -D today="$(BUILD_DATE)" \


"BUILD_DATE" if it works wrt reproducible builds, then thanks for teaching me something new!
I used to dpkg-parsechangelog to get a timestamp date

"rm -rf -- ./$(PYBUILD_NAME).egg-info"

I think this should be done automatically by the clean target.

I would just remove the _build and nothing more.

3) lintian override: you can keep it if you want, as you wish

4) copyright:

this is the tricky part as usual :)

please use the Debian packages instead of embedding external libraries
(I didn't check the below)
collateindex.pl
docbook-dsssl: /usr/bin/collateindex.pl
docbook-dsssl: /usr/share/man/man1/collateindex.pl.1.gz

apt-file search ldp.dsl
ldp-docbook-dsssl: /usr/share/sgml/docbook/stylesheet/dsssl/ldp/ldp.dsl


apt-file search tldp-one-page.xsl (and the others)
ldp-docbook-xsl: /usr/share/xml/docbook/stylesheet/ldp/html/tldp-one-page.xsl

can't you just use them from the system?
In case they differ, how about proposing patches to the actual Debian tools?

embedded stuff needs to be avoided as much as possible.

# Files: extras/dsssl/* extras/css/*
# Copyright: 2000-2003  - Greg Ferguson (gferg@metalab.unc.edu)
# License: GPL-2.0+
#
Files: extras/dsssl/* extras/xsl/* extras/css/*
Copyright: 2000-2002  - David Horton (dhorton@speakeasy.net)
License: GFDL-1.2


this is conflicting even if commented
(but using debian stuff, and removing them from the tarball will fix automagically the issue)

also commented stuff in copyright should be avoided to me
sample-documents: aren't on Debian?
what about packaging them?

why the license for them is commented?
https://codesearch.debian.net/results/License:%20GFDL-1.2/page_0
please try to add a copy of the license

the other stuff looks good, let me know how you intend to proceed with the above issues, and I'll happily
to another review

automatic checks (check-all-the-things package)
$ codespell --quiet-level=3
./extras/collateindex.pl:643: thier  ==> their

$ fdupes -q -r . | grep -vE '/(\.(git|svn|bzr|hg|sgdrawer)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s

$ grep -Er '/(home|srv|opt)(\W|$)' .

$ pep8 --ignore W191 .

$ perlcritic -1 . 2>&1 | grep -vF 'No perl files were found.'

$ pyflakes .

$ pyflakes3 .

$ grep -riE 'fixme|todo|hack|xxx' .

$ grep -r '/tmp/' .

$ Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata

$ find -type f -iname '*.xml' -exec xmllint --noout --nonet {} +


note: some of them are just nitpicks, and some others might be nice to fix, I leave checks to you

cheers,

G.


Reply to: