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@xxxxxx> 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review