https://bugzilla.redhat.com/show_bug.cgi?id=1536852
--- Comment #2 from Alexander Ploumistos <alex.ploumis...@gmail.com> ---
Hello Antonio and thanks for taking this review request.
(In reply to Antonio Trande from comment #1)
> - Are you sure that 'qca' (or qca-qt5?) is required for building?
Not really and neither was upstream. I commented it out, didn't notice any
problems. The OpenSUSE spec file also listed libqt5-qttools as a requirement
for the devel package, but we don't have that in Fedora. Do you know what that
is about?
> - Make arch-specific the 'openbabel' explicit request:
> Requires: openbabel%{?_isa}
Changed it, but why is that needed?
> - devel subpackage requires the main package (that contains libmolsketch.so):
> Requires: %{name}%{?_isa} = %{version}-%{release}
Fixed.
> - Remove that macros in comments or use %% for disabling them.
Did not know about the % escaping, did that.
> - Use %make_install in the %install section
Fixed.
> - Remove INSTALL as documentation
Removed.
> - %{_libdir}/lib*.so* installs unversioned libraries too. Change it with
> %{_libdir}/lib*.so.*
Fixed. Can you point me to any good resource regarding versioned-unversioned
libraries?
> - Regarding your question on devel mailing list, there could be a problem
> with some architectures (like s390x) about how to set correctly the
> libraries directory.
> In particular,
>
> obabeliface/obabeliface.pro
> libmolsketch/libmolsketch.pro
>
> contain a "DESTDIR = ../lib" line. To fix try to add the option
> ""MSK_INSTALL_LIBS=%{_libdir}" to qmake's line.
Thanks, it builds now.
> - Set MSK_INSTALL_INCLUDES to %{_includedir}/lib%{name}
I also changed
%files devel
%{_includedir}/%{name}
to
%files devel
%{_includedir}/lib%{name}
because the build failed complaining about a missing file.
> - /usr/share/icons/hicolor/scalable/mimetypes must not be owned by this
> package.
I declared both images in the %files section, perhaps I should just have gone a
level below %{_datadir}/icons/hicolor/scalable/ and used the wildcard there.
> - doc is a stand-alone package, it must provide an own license file.
Added.
> - Most files are licensed under GPLv2+ license; you can use it as License
> file i guess.
Changed to GPLv2+.
Spec URL: https://alexpl.fedorapeople.org/packages/Molsketch/molsketch.spec
SRPM URL:
https://alexpl.fedorapeople.org/packages/Molsketch/molsketch-0.5.1-3.fc28.src.rpm
koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=24370202
There is one serious regression now. When the program starts, I can see that it
is not using OpenBabel, in the bottom right corner it shows everything with a
minus (OpenBabel, InChI, gen2d). Can you tell where I've messed up?
--
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
