Bug#670448: RFS: bibtexconv/0.8.16-3 [ITP]
Thomas Dreibholz wrote:
> I have created an updated version here (version 0.8.19-1):
> http://mentors.debian.net/package/bibtexconv .
>
>
> > I took a look at your package:
> >
> > - Build fails in a clean chroot with g++-4.7:
> >
> > [...]
> > dh_auto_build
> > make[1]: Entering directory `/tmp/buildd/bibtexconv-0.8.16'
> > make all-recursive
> > make[2]: Entering directory `/tmp/buildd/bibtexconv-0.8.16'
> > Making all in src
> > make[3]: Entering directory `/tmp/buildd/bibtexconv-0.8.16/src'
> > g++ -DHAVE_CONFIG_H -I. -I.. -g -O2 -Wall -DUSE_UTF8 -c -o
> > bibtexconv.o bibtexconv.cc bibtexconv.cc: In function 'unsigned int
> > checkAllURLs(PublicationSet*, const char*, bool)': bibtexconv.cc:254:60:
> > error: 'unlink' was not declared in this scope bibtexconv.cc:285:42: error:
> > 'unlink' was not declared in this scope bibtexconv.cc:287:35: error:
> > 'unlink' was not declared in this scope make[3]: *** [bibtexconv.o] Error 1
> > make[3]: Leaving directory `/tmp/buildd/bibtexconv-0.8.16/src'
> > make[2]: *** [all-recursive] Error 1
> > make[2]: Leaving directory `/tmp/buildd/bibtexconv-0.8.16'
> > make[1]: *** [all] Error 2
> > make[1]: Leaving directory `/tmp/buildd/bibtexconv-0.8.16'
> > dh_auto_build: make -j1 returned exit code 2
> > make: *** [build] Error 2
>
> Fixed.
Indeed, builds fine for me now.
> > - In debian/copyright, the Format URL is wrong [1]. Although not
> > required, it wouldn't be a bad idea adding an Upstream-Contact
> > field.
> >
> > [1] http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
>
> Added.
>
>
> > ltmain.sh should have its own Files paragraph.
What about that^^^?
You also need to mention the OpenSSL exception (there's an example in
the "License specification->Syntax" section of [1].
[1] http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
And it's not clear whether GPL-3 or GPL-3+ applies; the synopsis
says GPL-3, but the license text indicates GPL-3+.
> >
> > - Since you only have one entry in the changelog, version number
> > should be 0.8.16-1.
>
> Fixed.
>
>
> > - You may want to use debhelper compat 9.
>
> Done.
You should specify the versioned dependency as ">= 9" instead of
">= 9.0".
> > - It looks like src/bibtexconv-odt doesn't use any bashisms, so you
> > could use /bin/sh.
>
> Changed.
>
>
> > That script will fail if $BIBTEX_FILE or $EXPORT_SCRIPT contain
> > spaces though.
>
> Fixed. It now also processed files with spaces in their names.
>
>
> > You would also get a more readable script (and less error-prone) by
> > using "set -e" [2]; for example, you're not checking the exit status
> > of mktemp.
>
> Done.
Good, but now it means you should change the "&&" construct. The script
will exit if any command fails, so you don't need the long "&&" chain;
instead, you should make sure that any command that is allowed to fail
is followed by "|| true" or something similar.
> > [2] http://www.debian.org/doc/debian-policy/ch-files.html#s-scripts
> >
> > - bibtexconv(1) states that "the following arguments have to be
> > provided"; it seems unlikely that all of the options are mandatory,
> > so this needs to be clarified. In the synopsis, the usual convention
> > is to put only optional arguments between brackets (the usage line
> > of src/bibtexconv-odt should follow that convention too).
>
> Changed.
You didn't change the usage line in src/bibtexconv-odt. And in
bibtexconv(1), I assume that the bibtex file is a mandatory argument, so
it shouldn't be between brackets in the synopsis line.
> > Some of the commands are not documented.
> >
> > The examples get wrapped in narrow terminals, in a way that make
> > them invalid shell commands; it would be better to manually wrap
> > them and have continuation lines end with '\'.
>
> Strangely, in the first occurrence of "\\" after .It, man converts "\" to "|"
> (i.e. the pipe symbol). I did not find a way to turn this wrong behaviour off,
> therefore I did not add manual wrapping.
"\e" will give you a backslash.
> > Do not use a pair of '*' for emphasis (e.g. "*not*"), there are
> > macros for that, such as ".I".
>
> Fixed.
>
>
> > Please consider getting rid of the AUTHOR section (see
> > man-pages(7)).
>
> Removed.
That's great, thanks.
Cheers,
--
Benoît Knecht
Reply to: