|Summary:||Merge Review: opensp|
|Product:||[Fedora] Fedora||Reporter:||Nobody's working on this, feel free to take it <nobody>|
|Component:||Package Review||Assignee:||Ville Skyttä <ville.skytta>|
|Status:||CLOSED RAWHIDE||QA Contact:||Fedora Package Reviews List <fedora-package-review>|
|Fixed In Version:||opensp-1.5.2-9.fc10||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|Last Closed:||2008-10-23 08:57:14 UTC||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
Description Nobody's working on this, feel free to take it 2007-01-31 20:19:05 UTC
Fedora Merge Review: opensp http://cvs.fedora.redhat.com/viewcvs/devel/opensp/ Initial Owner: email@example.com
Comment 1 Ondrej Vasik 2007-07-27 08:23:12 UTC
Package Change Request ====================== Package Name: opensp Updated Fedora Owners: firstname.lastname@example.org
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 http://sourceforge.net/projects/openjade/ 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 sf.net License category is a bit inaccurate. Compare COPYING in the OpenSP tarball with https://fedoraproject.org/wiki/Licensing/MIT and https://fedoraproject.org/wiki/Licensing/BSD
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.