Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 226217

Summary: Merge Review: opensp
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Ville Skyttä <ville.skytta>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: ovasik, ville.skytta
Target Milestone: ---Flags: ville.skytta: fedora-review+
wtogami: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: opensp-1.5.2-9.fc10 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-10-23 08:57:14 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Description Flags
Drop *.la, fix License tag, update comments none

Description Nobody's working on this, feel free to take it 2007-01-31 20:19:05 UTC
Fedora Merge Review: opensp
Initial Owner:

Comment 1 Ondrej Vasik 2007-07-27 08:23:12 UTC
Package Change Request
Package Name: opensp
Updated Fedora Owners:

Comment 2 Ville Skyttä 2008-10-22 16:36:04 UTC
The package is in a good shape, the only things I think should be done are:

- Drop *.la,
- License: MIT instead of BSD, and
- %check comments could be updated.

Will attach a patch.

Comment 3 Ville Skyttä 2008-10-22 16:36:45 UTC
Created attachment 321176 [details]
Drop *.la, fix License tag, update comments

Comment 4 Ondrej Vasik 2008-10-22 17:19:50 UTC
Thanks for review...
Ok with droping .la and comments, but about license I have a question - as upstream has BSD license on URL page. How you got the feeling that MIT is more suitable for OpenSP?

Comment 5 Ville Skyttä 2008-10-22 18:28:16 UTC
Yes, I think upstream's License category is a bit inaccurate.  Compare COPYING in the OpenSP tarball with and

Comment 6 Ondrej Vasik 2008-10-22 19:14:22 UTC
Ok, looks exactly as Modern Style of MIT ... therefore will use MIT, thanks for explanation. Built in Rawhide as opensp-1.5.2-8.fc10.

Comment 7 Ville Skyttä 2008-10-22 19:44:25 UTC
One remaining cosmetic thing I noticed but failed to note in previous comments:

$ rpmlint -i opensp-1.5.2-8.fc9.x86_64.rpm
opensp.x86_64: W: file-not-utf8 /usr/share/doc/opensp-1.5.2/ChangeLog
The character encoding of this file is not UTF-8.  Consider converting it in
the specfile for example using iconv(1).

Not at all a blocker but would be nice to get it fixed nevertheless.  Setting approved already, feel free to close when you've looked into it.

Comment 8 Ondrej Vasik 2008-10-23 08:57:14 UTC
Fixed in opensp-1.5.2-9.fc10, I will ignore last remaining rpmlint warning about missing doc files in opensp-devel subpackage as the documentation is in main opensp package. Closing RAWHIDE, thanks again for review.

Comment 9 Ville Skyttä 2008-10-23 14:53:01 UTC
The conversion should not be done in %build because that will result it being done multiple times and thus in corrupted result when doing multiple successive --short-circuit builds.  %prep would be the proper place to do it (I'll update rpmlint's message to reflect that).

Also, there's little reason to convert HTML files that have the correct encoding in their meta info - HTML files are opened with browsers that should grok the encoding just fine.  This is the reason why rpmlint does not complain about HTML (and some other) files' encodings even when they're not UTF-8.

Comment 10 Ondrej Vasik 2008-10-24 07:25:15 UTC
Thanks, ok, have to upgrade my rpmlint, because my version complained about ".html" file as well - and because it was created at build time, I used iconv in build section. Will move it to prep again without releasenotes.html conversion.