Bug 226088 - Merge Review: libxslt
Merge Review: libxslt
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
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 14:33 EST by Nobody's working on this, feel free to take it
Modified: 2010-10-28 01:40 EDT (History)
4 users (show)

See Also:
Fixed In Version: libxslt-1.1.26-5.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-10-28 01:40:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+


Attachments (Terms of Use)
spec cleanup (4.68 KB, patch)
2010-10-11 05:34 EDT, Parag AN(पराग)
no flags Details | Diff
More careful approach to charset conversion (4.23 KB, patch)
2010-10-22 13:20 EDT, Paul Howarth
no flags Details | Diff

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

http://cvs.fedora.redhat.com/viewcvs/devel/libxslt/
Initial Owner: veillard@redhat.com
Comment 1 Parag AN(पराग) 2010-10-11 05:34:17 EDT
Created attachment 452672 [details]
spec cleanup

Please commit this git patch to clean this package as per packaging guidelines or allow to commit it.
Comment 2 Parag AN(पराग) 2010-10-11 06:15:52 EDT
Following are the changes proposed in above patch
1) Generally we used to have dependent packages already built in repo so I
guess no need of versioned BuildRequires: and also Requires:

See,https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

If this package needs versioned BR: and R: then please add comment in spec file

2) Guidelines suggests to keep timestamps of upstream installed files. So
please use 
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"

See https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

3) Guidelines suggests package built above F-13 do not need %clean

See https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean

4) Guidelines recommends defattr usage as 
%defattr(-, root, root,-)
See https://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions

5) rpmlint warned about non-utf8 file message so used http://fedoraproject.org/wiki/Packaging_tricks#Convert_encoding_to_UTF-8

6) Used current macros defined -> http://fedoraproject.org/wiki/Packaging:RPMMacros

7) tried to clean this spec as per /etc/rpmdevtools/spectemplate-lib.spec

8)  we don't want to provide private python extension libs
https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering
Comment 3 Parag AN(पराग) 2010-10-21 02:26:21 EDT
committed patch but removed filtering. As per guildelines, this is arch package and installs binary in /usr/bin and also have libs.
Comment 4 Parag AN(पराग) 2010-10-22 00:35:55 EDT
Committed the above patch and built in libxslt-1.1.26-4.fc15

APPROVED.
Comment 5 Paul Howarth 2010-10-22 13:20:47 EDT
Created attachment 455147 [details]
More careful approach to charset conversion

tutorial2/libxslt_pipes.xml is explicitly iso-8859-2 encoded and shouldn't have been converted to UTF-8 using iconv.

Attached patch does the conversion of the other docs without iconv and is a more surgical approach.
Comment 6 Parag AN(पराग) 2010-10-25 00:23:21 EDT
Can you help me to get above patch updated for 1.1.26 release? this upstream release has been already available in all fedora branches.

So, we need not use iconv for xml files. Can we also need to convert NEWS to utf8?
Comment 7 Paul Howarth 2010-10-27 08:14:51 EDT
The patch applies cleanly against 1.1.26 and handles NEWS already.
Comment 8 Parag AN(पराग) 2010-10-27 11:53:09 EDT
Sorry and thanks. Patch is working fine. Actually I got some encoding issues on my machine that is why patch was not working.
Comment 9 Parag AN(पराग) 2010-10-28 01:40:37 EDT
Built in libxslt-1.1.26-5.fc15

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