[Bug 225999] Merge Review: libdrm

[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=225999


Orcan 'oget' Ogetbil <oget.fedora at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |oget.fedora at gmail.com
               Flag|                            |fedora-review?




--- Comment #9 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-01-31 19:46:12 EDT ---
OK, here's the review, with questions (?), issues (*) and comments (!).

? From what you told, I understand that you are the upstream maintainer too. So
why are the patches? This confuses me. Can't they be integrated into the
source? Also why use autoreconf?

? Why are those header files are getting removed? And if they are irrelevant,
why are being installed by the Makefile? An explanation please, preferably in
the SPEC file as a comment.

* Generally, all the patches need to be explained as comments in the SPEC file
(and they need to be sent upstream but we skip this part). It's best to keep
the SPEC file at a state where a new package maintainer can take it over easily
without spending hours to figure out what's going on.

* Now, the rpmlint complaints:
    libdrm.src: W: patch-not-applied Patch2:
libdrm-2.4.0-no-freaking-mknod.patch
should be removed if it's useless
    libdrm.src: W: strange-permission make-git-snapshot.sh 0755
we should have 644 for source files
    libdrm.x86_64: W: non-conffile-in-etc
/etc/udev/rules.d/91-drm-modeset.rules
this %files entry should be a %config
    libdrm-devel.x86_64: W: no-documentation
at the least, the file "libdrm/ChangeLog" and the "tests" directory can go in
here.

* %description should be descriptive, not a carbon copy of the summary.

* libdrm/TODO should go to %doc

* The upstream should be advised to put a copy of the full text of the license
in a seperate COPYING file.

! The timestamp of the sourcefile is wrong (should be downloaded with "wget -N"
or such)

! BR: pkgconfig is not required since libxcb-devel will pull that up.

* /etc/udev/rules.d is not owned. So we must require the package that owns it
(which, I think, is udev).

* Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for
directory ownership and usability). This applies to the devel package.

* Parallel make must be supported whenever possible. If it is not supported,
this should be noted in the SPEC file as a comment.

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