[Bug 622314] Review request: 3Depict- Valued point cloud visualisation and analysis

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=622314

--- Comment #5 from Martin Gieseking <martin.gieseking at uos.de> 2010-09-20 17:17:46 EDT ---
Hi D Haley and Damian,

Since I'm sponsoring Damian, here are some comments on his remarks. :)

> Just one point concerning the summary comment; the idea is to show the 
> program does not just work on point clouds, but rather point clouds with an
> associated scalar value (hence "valued point clouds") -- any ideas how this 
> could be improved?

I'm not a native English speaker, but maybe something like
"Application to visualize and analyze valued 3D point clouds"
could be a possible variant.


Damian, I agree with most of your comments. Here are just a couple of
additional notes and corrections.


> - MUST: The package MUST successfully compile and build into binary rpms on at
> least one primary architecture. 
> 
> Frankly speaking it compiles, but only thanks to the following hack in the spec
> file:
> export LDFLAGS="-lGL -lpng"
> which should be removed and the configure itself using appropriately pkg-config
> tool should compose compiler/linker command line adequately.
> NOT OK

These are things to be fixed by upstream, and the packager usually don't need
to address them as long as a simple addition of explicit LDFLAGS leads to a
working binary RPM. But since D Haley is the upstream developer, he might want
to fix these issues anyway.



> - MUST: If the package does not successfully compile, build or work ...
> Ditto.
> NOT OK.

The package compiles, builds and works properly, so this MUST item is OK.


> - MUST: All build dependencies must be listed in BuildRequires, except for any
> that are listed in the exceptions section of the Packaging Guidelines ;
> inclusion of those as BuildRequires is optional. Apply common sense.
> 
> There is no BR: for libpng.
> NOT OK.

BR: libpng-devel is not required as it's added as an indirect dependency of
wxGTK-devel. If a package builds in mock/koji, usually all necessary BRs are
present.


> %{_mandir}/man1/%{name}.1.gz

The .gz suffix of manpages should be avoided since the compression format
applied by rpmbuild might change. Thus, %{_mandir}/man1/%{name}.1* is more
appropriate.


> - MUST: Packages containing GUI applications must include a %{name}.desktop
> file, and that file must be properly installed ...

> NOT OK. See note about macro consistency.

The .desktop file is present and it is installed properly with
desktop-file-install. => OK so far. 
However, the icon "3Depict" referenced in the .desktop file is missing. It
should be added to the package.

Finally, the spec file should get a short comment above Patch0 describing the
purpose of the patch.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.