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

Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.



Am 06.12.2015 um 21:13 schrieb Alexandre Detiste:
> Le dimanche 6 décembre 2015, 20:34:42 Markus Koschany a écrit :
[...]
>> You build-depend on wx2.8-headers but this package is obsolete and will
>> be removed soon. Please either use wx3.0-headers instead or remove the
>> build-dependency because I don't see anything in the sources that may
>> need it.
> 
> find . -type f -exec grep ^#include {} \; | grep wx | sort -u
> #include <wx/app.h>
> #include <wx/bitmap.h>
> #include <wx/button.h>
> #include <wx/checkbox.h>
> #include <wx/dcclient.h>
> #include <wx/dcmemory.h>
> #include <wx/dirdlg.h>
> #include <wx/dir.h>
> #include <wx/filedlg.h>
> #include <wx/file.h>
> #include <wx/filename.h>
> #include <wx/frame.h>
> #include <wx/glcanvas.h>
> #include <wx/image.h>
> ...
> 
> It's used in MapEdit/ (currently disabled) and AnimView/
> I need to check if this second direcotry is a depedency of MapEdit only or
> if it is used somewhere else.

I have just rebuilt the package without the B-D on wx2.8-headers and it
still works.

>> Why do you depend on lua-filesystem and lua-lpeg explicitly? If those
>> dependencies are really required, the ${shlibs:Depends} substvar should
>> include them already. Otherwise the build system should be updated to
>> require and incorporate those libraries.
> 
> Because they are only needed at runtime...
> does they really need to get pulled in the build chroot for nothing ?
> I guess it's cleaner that way.

No, if they are only needed at runtime, so be it.

>> desktop file: I have already fixed a few things that bothered me. I
>> believe it's a simulation game and StartupNotify should be set to false.
>> I also added keywords and a comment in German. Please tell me if you can
>> live with the changes or if there is a reason to stick with the old
>> values. Please forward the new desktop file upstream.
> 
> Upstream doesn't ship a .png icon either and had removed
> the non-maintained DebianPackager... 
> 
> It's not the kind of thing I really want to bother an upstream with; 
> ut rather more usefull things, like having appropriate configure
> scripts to avoid the need to patch the build system.

I saw that they provided a desktop file within the DebianPackager
directory. desktop files should definitely be provided with the upstream
sources because they are not Debian specific. Other distributions would
also benefit from this change.

> 
>> Please use Debian's system library of tinyxml. The embedded copy of
>> tinyxml in AnimView/ is neither mentioned in upstream's LICENSE.txt file
>> nor in debian/copyright.
> 
> As stated for WX, I need to check if AnimView is needed.
> 
>> Please ask upstream to remove the embedded copy
> That's downstream work.

No, it is not. Please point them to
https://wiki.debian.org/UpstreamGuide#No_inclusion_of_third_party_code

Markus

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: