[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Pavel Zhukov  changed:

   What|Removed |Added

 Status|NEW |CLOSED
 Resolution|--- |WONTFIX
Last Closed||2018-06-18 04:21:52



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/YAJXVPULTWV2BEN52ZLMES73YZWXSMNX/

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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



--- Comment #8 from Tarjei Knapstad  ---
Sorry for not responding sooner, this took a backseat among a lot of other
responsibilities.

I agree that this can be closed:

1. The author recommends transitioning to NNG from nanomsg
2. There are no packages in Fedora that requires nanomsg.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/XN3X4BLODB5JXD4SG5TLP73UVHWH6T3U/

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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



--- Comment #7 from Pavel Zhukov  ---
I think this can be closed as reporter is non responsible and we have new RR
BZ#1592138

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/RMWLNMBK4CLM3DLYI7RZRJUKUE4Z65RV/

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Pavel Zhukov  changed:

   What|Removed |Added

 Blocks||201449 (FE-DEADREVIEW)



--- Comment #6 from Pavel Zhukov  ---
I think this can be closed as reporter


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=201449
[Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter
response should be blocking this bug.
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/KB7XPJVYBPHSCKQ3FKIIIWCF2K2YP2PP/

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Peter Robinson  changed:

   What|Removed |Added

 CC||pbrobin...@gmail.com



--- Comment #5 from Peter Robinson  ---
Related: I packaged up the now stable nng (nanomsg next generation) - rhbz
1592138

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/HPEOEDTSFJWDEE56NJUUQGGL5YMFBVH3/

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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



--- Comment #4 from Michael Schwendt  ---
> I don't think that V=1 is also needed

The older review request bug 1123511 included version 0.7.0 of the program,
which wasn't using CMake. This new request is about 1.0.0.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Neal Gompa  changed:

   What|Removed |Added

  Flags||needinfo?(ignatenko@redhat.
   ||com)



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Neal Gompa  changed:

   What|Removed |Added

 CC||ngomp...@gmail.com



--- Comment #3 from Neal Gompa  ---
I can sponsor if Igor takes on reviewing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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



--- Comment #2 from Igor Gnatenko  ---
Hello,

unfortunately I can't sponsor you, but I will review your package and will try
to help making it ready for inclusion into Fedora.

First thing is Spec/SRPM URLs, make sure that you point them to direct files,
for example your spec url leads to html page with rendered spec file and srpm
just shows copr repository. So you want to link
https://gist.githubusercontent.com/tknapstad/6c8baca1678307acfbea59ad1b59da13/raw/7436f267b19197bb0ed627382aed977bced774cd/nanomsg.spec
as a spec file and
https://copr-be.cloud.fedoraproject.org/results/tknapstad/nanomsg/fedora-rawhide-x86_64/00535509-nanomsg/nanomsg-1.0.0-1.fc27.src.rpm
as a srpm file. This is needed for fedora-review tool and also simplifies life
for reviewer.

Now let's go through spec file, I will comment inline:

> Group: System Environment/Libraries
> BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XX)
> %clean
No need to include this sections/tags, it's mostly needed for EL5 compatibility
which I guess is not your case
See: https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

> Summary: A fast, scalable, and easy to use socket library
My personal preference is to remove such "A" or "The" since they don't make
sense and just consuming space =) Probably there are guidelines about this, but
I can't find them right now.

> Source0: https://github.com/nanomsg/nanomsg/archive/%{version}.tar.gz
Having tarball names like 1.0.0.tar.gz is not really useful, it is better to
include name in there, so SourceURL could be like
https://github.com/nanomsg/nanomsg/archive/%{version}/%{name}-%{version}.tar.gz
See: https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Tags

> BuildRequires: rubygem-asciidoctor xmlto cmake
Personally I prefer each dependency to be on its own line because this
simplifies git diffs a lot and makes rebasing / cherry-picking way easier.
Also you miss BuildRequires: gcc because nanomsg uses it to build =)
See: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires

> %setup -q -n %{name}-%{version}
Note that "-n %{name}-%{version}" is default for %*setup macro, so you can
safely remove this. Also I would recommend you to change whole line to
"%autosetup" which also would apply patches automatically if you have them
(P.S. if you will have problems with applying patches, you might want to add
"-p1" after)

> make %{?_smp_mflags} V=1
You could replace (and I recommend to) this with %make_build (I don't think
that V=1 is also needed because %cmake macro automatically adds
-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON to cmake invocation which makes make verbose)

> rm -fR %{buildroot}
This is not needed, buildroot is removed automatically in modern rpm versions

> make install DESTDIR="%{buildroot}"
You could replace (and I recommend to) this with "%make_install" which does
absolutely same

> make test LD_LIBRARY_PATH="%{buildroot}%{_libdir}" DESTDIR="%{buildroot}"
I don't think that DESTDIR is required here..

> %defattr(-,root,root)
You don't need to declare this line because it is default for RPM
See: https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

> %doc AUTHORS COPYING README
You should use %license for referencing licenses, so change this line to:
%license COPYING
%doc AUTHORS README
See: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines

> %{_libdir}/*.so.*
> %{_libdir}/*.a*
> %{_mandir}/man7/*
> %{_mandir}/man3/*
> %{_includedir}/*
> %{_libdir}/*.so
> %{_libdir}/pkgconfig/*.pc
> %{_mandir}/man1/*
> %{_bindir}/*
I would recommend you changing this to reflect real names of files, it is hard
to guess what files are there and might become problem when upstream adds new
binary / file and you just don't notice this..

> %{_docdir}/%{name}
While this works on modern Fedoras, I would recommend you to change cmake
definitions to not install it automatically and use %doc to install
documentation. The reason is on (for example) RHEL, docdir is
%{name}-%{version} while on Fedora it is %{name}.

---

Also you missed "Requires: %{name}%{?_isa} = %{version}-%{release}" in the
"utils" subpackage, it links with libnanomsg.so and you need to keep subpackage
in sync.

I also noticed that "static" bcond doesn't really work: File not found:
/home/brain/rpmbuild/BUILDROOT/nanomsg-1.0.0-1.fc28.x86_64/usr/lib64/*.a*
And since we don't really want static libraries in Fedora, I would recommend
you to drop bcond or at least fix it.

Another thing which needs to be fixed is pkg-config file having
"libdir=${prefix}/lib" which on 64-bit platforms should have lib64 instead.

===

I hope you will find my review useful and I'm very sorry that no one reviewed
your simple package for half year =(

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Michael Schwendt  changed:

   What|Removed |Added

 Blocks|201449 (FE-DEADREVIEW)  |




Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=201449
[Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter
response should be blocking this bug.
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Michael Schwendt  changed:

   What|Removed |Added

 Blocks||201449 (FE-DEADREVIEW)
 CC||cleaver-redhat@terabithia.o
   ||rg



--- Comment #1 from Michael Schwendt  ---
*** Bug 1123511 has been marked as a duplicate of this bug. ***


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=201449
[Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter
response should be blocking this bug.
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1438927] Review Request: nanomsg - A fast, scalable, and easy to use socket library

[bugzilla]

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

Tarjei Knapstad  changed:

   What|Removed |Added

 Blocks||177841 (FE-NEEDSPONSOR)




Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org