[Bug 1012392] Review Request: nanomsg - nanomsg is a socket library that provides several common communication patterns

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1012392

Zoltan Boszormenyi <zboszor@xxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|package-review@lists.fedora |
                   |project.org                 |



--- Comment #5 from Zoltan Boszormenyi <zboszor@xxxxx> ---
(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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=9j76j9WJmM&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]