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

Bug#799768: RFS: ismrmrd/1.3.1-1





On 22/09/15 14:57, Gianfranco Costamagna wrote:
Control: merge -1 799767
Control: owner -1 !

Hi Ghislain,


the packaging looks good, however I have some nitpicks/issues:


rules file:

"-DCMAKE_INSTALL_PREFIX=/usr"

is this really needed? AFAIK it should be the default inside a dh build.
(please check)

"--builddirectory=$(BUILDDIR)"

same here

(this will simplify a lot the packaging)
"cd $(BUILDDIR) && $(MAKE) apidoc"

make -C is your friend :)

something like $(MAKE) -C obj-* apidoc should work
(untested)


Indeed, none of CMAKE_INSTALL_PREFIX, CMAKE_BUILD_TYPE and customized BUILDDIR are required.

This simplifies d/rules quite a lot. I'll make sure to apply this knowledge to other packages I maintain.

now a real issue: please move that to a build-indep target, and add
the required build-dependencies to build-depends-indep target
(you might want to look at lucene++, a similar package I maintain)

Great suggestion.

to check if it is done correctly you can run dpkg-buildpackage -A
and dpkg-buildpackage -B
(this will allow you to do source-only uploads)

control:
move doc stuff to build-depends-indep
Vcs-Git has a space at the end
(not an issue but vim shows it in red colour)

changelog: much stuff is not mentioned:
- a new patch
- a patch dropped

- change of the documentation package installation directory
- drop of a ${shlibs:Depends} (why?)

I will make sure these are more detailed.

last personal note:
"usr/share/javascript/jquery/jquery.js usr/share/doc/libismrmrd-doc/html/api/jquery.js"

well I would like to avoid using the embedded jquery, read e.g.
https://lists.debian.org/debian-devel/2014/10/msg00783.html

using the system jquery instead of the doxygen patched one might result in a broken documentation.

(the link I gave you should have references to Debian bugs and other discussions)

I still don't get what the recommended solution is. So far, I have been (wrongly?) following what Lintian suggested to do with embedded jquery.

Ghislain


Reply to: