Fedora Merge Review: libXinerama http://cvs.fedora.redhat.com/viewcvs/devel/libXinerama/ Initial Owner: sandmann
The full review arrived! * Summary and especially the description are bizarre. Can you update them. You can find these on the manpage: Xinerama - API for Xinerama extension to X11 Protocol Xinerama is a simple library designed to interface the Xinerama Extension for retrieving information about physical output devices which may be combined into a single logical X screen. * rpmlint says libXinerama.src:18: W: unversioned-explicit-obsoletes XFree86-libs libXinerama.src:18: W: unversioned-explicit-obsoletes xorg-x11-libs libXinerama.src:32: W: unversioned-explicit-obsoletes XFree86-devel libXinerama.src:32: W: unversioned-explicit-obsoletes xorg-x11-devel libXinerama.x86_64: E: zero-length /usr/share/doc/libXinerama-1.0.3/AUTHORS libXinerama.x86_64: E: zero-length /usr/share/doc/libXinerama-1.0.3/README libXinerama.x86_64: W: obsolete-not-provided XFree86-libs libXinerama.x86_64: W: obsolete-not-provided xorg-x11-libs libXinerama-devel.x86_64: W: obsolete-not-provided XFree86-devel libXinerama-devel.x86_64: W: obsolete-not-provided xorg-x11-devel The zero-length files are obviously not needed so they should be removed. The obsoletes look very problematic. Can you fix those (or alternatively explain them in the SPEC file as comments)? * BR: libXau-devel is not needed. Afaict it is not used. * BRs: libX11-devel pkgconfig and xorg-x11-proto-devel are not needed. They will be picked up by libXext-devel. * Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). This applies to the devel package. ! Try to make use of the %{name} macro (e.g. files sections). * Parallel make must be supported whenever possible. If it is not supported, this should be noted in the SPEC file as a comment. Added Adam and Matthias to CC since they are the last two known maintainers. Sorry if this was not desired.
Mostly fixed in 1.0.3-3, except for the %description bit Note that xorg-x11-proto-devel pulls in pkgconfig for free.
Thanks, there are two more things. ? Could you at least define xinerama in the description as Jason did in libdmx? This is just a request from me. * You forgot this one: BR: libXau-devel is not needed. Afaict it is not used.
ping?
ping again?
done.
where is the official review?
see comment 1, and then comment 3. It's all good. The packager should have informed me about the changes and waited for me to set the fedora-review flag to + though.
Parag, can we close this now since a full review has been done?
Yes please. Thanks for this review. But, I generally suggest Package change happened in F15 so, 1) buildroot should be removed 2) %clean not needed anymore 3) cleaning of buildroot at start of %install also not needed I wonder if maintainer can find some time to clean his package then why can't he apply above cosmetic changes also.
(In reply to comment #10) > 1) buildroot should be removed Any references for this? I don't see a guideline that says buildroot should be removed. > 2) %clean not needed anymore > 3) cleaning of buildroot at start of %install also not needed > 'Not needed' doesn't mean 'Must be removed'. It is the packager's choice to keep them. We have to respect that.
(In reply to comment #11) > (In reply to comment #10) > > 1) buildroot should be removed > > Any references for this? I don't see a guideline that says buildroot should be > removed. My understanding to reference http://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot_tag is that if buildroot tag is ignored then why to unnecessarily write them in spec files. > > 2) %clean not needed anymore reference to http://fedoraproject.org/wiki/PackagingGuidelines#.25clean > > 3) cleaning of buildroot at start of %install also not needed Again reference to http://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot_tag this cleaning of buildroot is now implicit. > > > > 'Not needed' doesn't mean 'Must be removed'. It is the packager's choice to > keep them. We have to respect that. Sure that is why I suggest them. So its upto maintainer whether to accept above 3 changes or not.
Thank you. I am sure the maintainer will consider your suggestions. Closing the bug for good this time.