Bug 226075 - Merge Review: libXinerama
Summary: Merge Review: libXinerama
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Orcan Ogetbil
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:30 UTC by Nobody's working on this, feel free to take it
Modified: 2010-09-02 06:49 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2010-09-02 06:49:00 UTC
Type: ---
Embargoed:
oget.fedora: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:30:58 UTC
Fedora Merge Review: libXinerama

http://cvs.fedora.redhat.com/viewcvs/devel/libXinerama/
Initial Owner: sandmann

Comment 1 Orcan Ogetbil 2009-01-17 23:05:35 UTC
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.

Comment 2 Adam Jackson 2009-02-21 21:03:13 UTC
Mostly fixed in 1.0.3-3, except for the %description bit

Note that xorg-x11-proto-devel pulls in pkgconfig for free.

Comment 3 Orcan Ogetbil 2009-02-22 02:03:08 UTC
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.

Comment 4 Orcan Ogetbil 2009-06-09 19:03:04 UTC
ping?

Comment 5 Orcan Ogetbil 2010-02-19 07:51:58 UTC
ping again?

Comment 6 Adam Jackson 2010-08-30 19:50:50 UTC
done.

Comment 7 Parag AN(पराग) 2010-08-31 01:38:13 UTC
where is the official review?

Comment 8 Orcan Ogetbil 2010-08-31 04:12:05 UTC
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.

Comment 9 Orcan Ogetbil 2010-08-31 22:32:30 UTC
Parag, can we close this now since a full review has been done?

Comment 10 Parag AN(पराग) 2010-09-01 10:36:12 UTC
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.

Comment 11 Orcan Ogetbil 2010-09-01 14:16:26 UTC
(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.

Comment 12 Parag AN(पराग) 2010-09-01 14:31:18 UTC
(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.

Comment 13 Orcan Ogetbil 2010-09-02 06:49:00 UTC
Thank you. I am sure the maintainer will consider your suggestions.

Closing the bug for good this time.


Note You need to log in before you can comment on or make changes to this bug.