Bug 1012392
Summary: | Review Request: nanomsg - nanomsg is a socket library that provides several common communication patterns | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Zoltan Boszormenyi <zboszor> |
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
Status: | CLOSED DUPLICATE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | cleaver-redhat, i, karlthered, mail, tomspur |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2014-07-25 10:41:47 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Zoltan Boszormenyi
2013-09-26 11:53:42 UTC
Koj koji scratch builds were completed for f20, f19, f18 and dist-6E-epel dist-5E-epel failed for some reason very early $ rpmlint -i -v nanomsg.spec nanomsg.spec: I: checking-url http://download.nanomsg.org/nanomsg-0.2-alpha.tar.gz (timeout 10 seconds) 0 packages and 1 specfiles checked; 0 errors, 0 warnings. CCing cuz I've packaged it. Informal review: 1. Summary: nanomsg is a socket library that provides several common communication patterns. Bad. Suggestion: Summary: A socket library which provides several common communication patterns 2. Remove BuildRoot tag. 3. It's nonsense to Requires: pkgconfig in -devel subpackage. 4. Why do we need static libs? If you don't recommend using it, you can drop it, or there really have people like static linking? 5. devel subpackage has: Requires: %{name} = %{version}-%{release} Should use: Requires: %{name}%{?_isa} = %{version}-%{release} 6. Put %check after %install 7. Remove rm -rf $RPM_BUILD_ROOT in %install 8. Why rm -f $RPM_BUILD_ROOT%{_infodir}/dir ? 9. Remove %clean section. 10. Remove %defattr(-,root,root,-) 11. Use %doc to define which are docs. 12. %dir %{_includedir}/%{name} %{_includedir}/%{name}/*.h Just %{_includedir}/%{name} is the best. 13. I don't think you need to say "It is licensed under MIT/X11 license." in %description, Fedora only allows OPEN SOURCE projects. (In reply to Christopher Meng from comment #4) > CCing cuz I've packaged it. > > Informal review: > > 1. Summary: nanomsg is a socket library that provides several common > communication patterns. > > Bad. > > Suggestion: > > Summary: A socket library which provides several common communication > patterns Done. > > 2. Remove BuildRoot tag. Done. > > 3. It's nonsense to Requires: pkgconfig in -devel subpackage. Done. > 4. Why do we need static libs? If you don't recommend using it, you can drop > it, or there really have people like static linking? Removed the static subpackage. > 5. devel subpackage has: > > Requires: %{name} = %{version}-%{release} > > Should use: > > Requires: %{name}%{?_isa} = %{version}-%{release} Done. > 6. Put %check after %install Done. > 7. Remove rm -rf $RPM_BUILD_ROOT in %install Done. > 8. Why rm -f $RPM_BUILD_ROOT%{_infodir}/dir ? It was there because I used another Fedora package as a start. Removed. > > 9. Remove %clean section. Done. > 10. Remove %defattr(-,root,root,-) Done. > 11. Use %doc to define which are docs. There was already %doc AUTHORS ChangeLog COPYING README in the main package. "make install" installs man pages into %{_mandir} and *.html pages (generated from the same source as the man pages) into %{_docdir}/%{name}. I marked the *.html as %doc in the devel subpackage. > 12. %dir %{_includedir}/%{name} > %{_includedir}/%{name}/*.h > > Just %{_includedir}/%{name} is the best. Done. > 13. I don't think you need to say "It is licensed under MIT/X11 license." in > %description, Fedora only allows OPEN SOURCE projects. I removed this last statement, it's redundant with "License:" anyway. I copied the description from http://nanomsg.org/index.html The new version is at the same URL as above: Spec URL: http://boszormenyi.dyndns.biz/nanomsg.spec SRPM URL: http://boszormenyi.dyndns.biz/nanomsg-0.2-3alpha.fc19.src.rpm I was planning to package it but i got delayed by an event as fp.o ambassador (where i gave a talk on nanomsg:) ) Good point, it passed the scratch build test: http://koji.fedoraproject.org/koji/taskinfo?taskID=6029041 I'm a sponsor, i can take care of that part if you agree. For a beginning, start by doing two informal reviews (please, put me in CC) and link them in this ticket. I'll also drill you on some fedora packaging guidelines, either by mail or irc. 1. Group tags are not required anymore, remove them 2. Release tag is not handled correctly for a pre-release, it should be something like %global alphatag alpha Release: O.3%{alphatag} Please read the following guideline: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages 3. for the url: http://nanomsg.org 4. Remove the requires on nanomsg in the nanocat subpackage, RPM can compute that dependency 5. i'd rather rename the nanocat subpackage into tools or utilities, other CLI might show up in the future.' 6. the description is way too long and not helpful for fedora users. Please *trim it* and make it *useful*. 7. Did you submit your patches upstream ? If you did, please provide a link to the upstream ticket, if you didn't, please send them. When you're done, put a comment on top of the patch with the url of the upstream ticket. 8. for the devel package, put the %doc first, ordering lines in %files section improves spec readability 9. You shouldn't specify the compression format for man pages, if we change it later, your spec will break. As i have a FPC member who backed me on that point so i'll make it a MUST ;) 10. for the Summary, i suggest: "A socket library for high-performance message-passing application" which is a more descriptive one-liner ping! Sorry it took me this long to get back to this task. (In reply to Haïkel Guémar from comment #6) > I was planning to package it but i got delayed by an event as fp.o > ambassador (where i gave a talk on nanomsg:) ) > Good point, it passed the scratch build test: > http://koji.fedoraproject.org/koji/taskinfo?taskID=6029041 > > I'm a sponsor, i can take care of that part if you agree. > For a beginning, start by doing two informal reviews (please, put me in CC) > and link them in this ticket. I'll do that later today or tomorrow. > I'll also drill you on some fedora packaging guidelines, either by mail or > irc. > > 1. Group tags are not required anymore, remove them Done. > 2. Release tag is not handled correctly for a pre-release, it should be > something like > %global alphatag alpha > Release: O.3%{alphatag} > Please read the following guideline: > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre- > Release_packages How about the git commit number and git date? I rebased to the current GIT version. > 3. for the url: http://nanomsg.org Fixed. > 4. Remove the requires on nanomsg in the nanocat subpackage, RPM can compute > that dependency Done. > 5. i'd rather rename the nanocat subpackage into tools or utilities, other > CLI might show up in the future.' Done. > 6. the description is way too long and not helpful for fedora users. Please > *trim it* and make it *useful*. Done. > 7. Did you submit your patches upstream ? If you did, please provide a link > to the upstream ticket, if you didn't, please send them. > When you're done, put a comment on top of the patch with the url of the > upstream ticket. I sent them long ago, but they were not appropriate, since nanomsg is also build with Visual Studio (via cmake) and it doesn't understand certain constructs. My patches were GCC-only. I have created a new patch, sent it upstream and created the issue: https://github.com/nanomsg/nanomsg/issues/208 My mail reporting the problem and the patch is at: http://www.freelists.org/post/nanomsg/Warning-fixes,7 > 8. for the devel package, put the %doc first, ordering lines in %files > section improves spec readability Done. > 9. You shouldn't specify the compression format for man pages, if we change > it later, your spec will break. As i have a FPC member who backed me on that > point so i'll make it a MUST ;) Done. > 10. for the Summary, i suggest: "A socket library for high-performance > message-passing application" which is a more descriptive one-liner Done. Spec URL: http://boszormenyi.dyndns.biz/nanomsg.spec SRPM URL: http://boszormenyi.dyndns.biz/nanomsg-0.2-0.3.20131217git624c483.fc20.src.rpm My dyndns address have been cancelled. New links: Spec URL: https://www.dropbox.com/s/h414giiyzc5scai/nanomsg.spec SRPM URL: https://www.dropbox.com/s/mu2qrn3czenf2pn/nanomsg-0.2-0.3.20131217git624c483.fc20.src.rpm My informal reviews: https://bugzilla.redhat.com/show_bug.cgi?id=970393#c7 https://bugzilla.redhat.com/show_bug.cgi?id=1036130#c9 (In reply to Zoltan Boszormenyi from comment #10) > My informal reviews: > https://bugzilla.redhat.com/show_bug.cgi?id=970393#c7 > https://bugzilla.redhat.com/show_bug.cgi?id=1036130#c9 You need to find a sponsor initiatively: https://admin.fedoraproject.org/accounts/group/members/packager/*/sponsor The latest release is 0.3 [1]. [1] http://nanomsg.org/download.html 0.4 was released on 6/10 http://download.nanomsg.org/nanomsg-0.4-beta.tar.gz *ping* I'd really love to see this included in Fedora. I'd be willing to take it (and this BZ) on if there's no other activity occurring. Last comment here: Zoltan, if you can't repond in 1 week I will close it. Japheth, I packaged this long time ago, and postponed by this needsponsor review. If you are a packager already, feel free to submit and I will review, if not I will do this. Fedora always lacks some packages just because of such reviews, I'm annoyed now. Thanks. Sorry for being non-responsive, I had to deal with a lot of other issues. I would be happy if someone else could pick this up, I can't invest time into this. Sorry to hear about that. I will talk with Japheth and continue this. *** This bug has been marked as a duplicate of bug 1123511 *** |