Bug 226066 - Merge Review: libXcursor
Merge Review: libXcursor
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 14:30 EST by Nobody's working on this, feel free to take it
Modified: 2009-12-29 01:57 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-12-29 01:57:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
panemade: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:30:03 EST
Fedora Merge Review: libXcursor

http://cvs.fedora.redhat.com/viewcvs/devel/libXcursor/
Initial Owner: sandmann@redhat.com
Comment 1 Parag AN(पराग) 2009-10-08 07:15:36 EDT
Review:
+ koji build =>http://koji.fedoraproject.org/koji/buildinfo?buildID=135668
+ verified upstream source as 
096d0e538d37fd865705e5f45b0e96c7294c1f2f  libXcursor-1.1.10.tar.bz2
+ rpmlint output can be ignored
libXcursor.i686: E: file-in-usr-marked-as-conffile /usr/share/icons/default/index.theme
+ Package libXcursor-1.1.10-2.fc13.i686
Provides: config(libXcursor) = 1.1.10-2.fc13 libXcursor.so.1
Requires: libX11.so.6 libXcursor.so.1 libXfixes.so.3 libXrender.so.1 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.1.3) libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) rtld(GNU_HASH)
+ Package libXcursor-devel-1.1.10-2.fc13.i686
Provides: pkgconfig(xcursor) = 1.1.10
Requires: /usr/bin/pkg-config libXcursor.so.1 pkgconfig(x11) pkgconfig(xfixes) pkgconfig(xproto) pkgconfig(xrender)

APPROVED.
Comment 2 Christoph Wickert 2009-10-12 13:10:40 EDT
Sorry, but you missed a couple of issues:

- md5sum is 7dcdad1c10daea872cb3355af414b2ca, both for upstream as well as for the file from lookaside cache
- timestamps not preserved during iconv
- timestamp of index.theme not preserved during install
- permissions are not 
- /usr/share/ hardcoded, although commented out
- devel package missing "Requires: xorg-x11-proto-devel"
- why do we still inherit Bluecurce in index.theme?
- make is not verbose, so RPM_OPT_FLAGS cannot be verified
  CC     cursor.lo
  CC     display.lo
  CC     file.lo
  CC     library.lo
  CC     xlib.lo
  CCLD   libXcursor.la

I know you are doing a lot of reviews, but IMO the quality must not suffer from the quantity.
Comment 3 Christoph Wickert 2009-10-12 13:11:13 EDT
(In reply to comment #2)

> - permissions are not 

Ignore that one.
Comment 4 Parag AN(पराग) 2009-10-12 23:13:48 EDT
(In reply to comment #2)
> Sorry, but you missed a couple of issues:
>
> - md5sum is 7dcdad1c10daea872cb3355af414b2ca, both for upstream as well as for
> the file from lookaside cache
  Thanks. I started using sha1sum

> - timestamps not preserved during iconv
  I wonder how can we preserve timestamps when using iconv. Can you please provide me an example?

> - timestamp of index.theme not preserved during install
   Thanks will update in SPEC.

> - permissions are not 
> - /usr/share/ hardcoded, although commented out
  will better remove comment then.

> - devel package missing "Requires: xorg-x11-proto-devel"
   Ok

> - why do we still inherit Bluecurce in index.theme?
   Need to check with maintainers.....

> - make is not verbose, so RPM_OPT_FLAGS cannot be verified
>   CC     cursor.lo
>   CC     display.lo
>   CC     file.lo
>   CC     library.lo
>   CC     xlib.lo
>   CCLD   libXcursor.la

  will take care of this.
> 
> I know you are doing a lot of reviews, but IMO the quality must not suffer from
> the quantity.  
Thanks. I know that but reason of doing these merge reviews in hurry is I was long time waiting for some time to give to review these but finally I decided to complete this as maintainers himself not interested for such minor thing changes in spec.

Feel free to cross verify my reviews :)

I really don't want to start again debate on why merge-reviews maintainers are not taking responsibility to change spec according to packaging guidelines.
Comment 5 Christoph Wickert 2009-10-13 04:41:32 EDT
(In reply to comment #4)
> (In reply to comment #2)
> > - md5sum is 7dcdad1c10daea872cb3355af414b2ca, both for upstream as well as for
> > the file from lookaside cache
>   Thanks. I started using sha1sum

Yeah, after the third mismatch I got it. The review Guidelines still state "The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task." If you use something different, please make notice in the bug.

> > - timestamps not preserved during iconv
>   I wonder how can we preserve timestamps when using iconv. Can you please
> provide me an example?

Second example on 
https://fedoraproject.org/wiki/PackageMaintainers/PackagingTricks#Convert_encoding_to_UTF-8

> Thanks. I know that but reason of doing these merge reviews in hurry is I was
> long time waiting for some time to give to review these but finally I decided
> to complete this as maintainers himself not interested for such minor thing
> changes in spec.

Sad but true, I know this very well from other merge reviews. Keep on your good work!
Comment 6 Parag AN(पराग) 2009-10-21 05:03:06 EDT
updated package in F-13

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