https://bugzilla.redhat.com/show_bug.cgi?id=1432983
Lubomir Rintel <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |[email protected] Assignee|[email protected] |[email protected] Flags| |fedora-review? --- Comment #1 from Lubomir Rintel <[email protected]> --- This looks rather well done. Just a couple of comments: 1.) Please unbundle cbang > %global commit1 9f80bae739a36727a5f66e5f1b0c55cd9ee1c01a > %global name1 cbang > %global shortcommit1 %(c=%{commit1}; echo ${c:0:7}) ... > Source1: > https://github.com/CauldronDevelopmentLLC/cbang/archive/%{commit1}.tar.gz#/%{name1}-%{shortcommit1}.tar.gz ... Looks like this should go into a separate package. See: https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplication_of_system_libraries 2.) Don't disable debuginfo > # With debug flags camotics executable does not run > %global debug_package %{nil} ... > # Trim ~2MB more, manually since not a debug build > %{__strip} %{buildroot}%{_bindir}/* Eeek! Don't do this. What didn't work here? It certainly works all right on my system. 3.) Submit the MIME info upstream > Source2: camotics.xml It's okay to add it in the package, but please ask upstream to include it and add a comment with a link to the upstream ticket. 4.) Why is this conditional on Fedora? > cd %{_builddir}/%{name1}-%{commit1} > scons ccflags="%{?fedora:-I%{_includedir}/v8-3.14/}" %{?_smp_mflags} > > cd %{_builddir}/CAMotics-%{version} > scons ccflags="%{?fedora:-I%{_includedir}/v8-3.14/}" %{?_smp_mflags} 5.) Please add BuildRequires: gcc-c++ The guindelines seem to mandate it now: https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B The rest of the review will follow. I'm a packager sponsor, so I can eventually sponsor you when we sort this review out. -- 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 -- [email protected] To unsubscribe send an email to [email protected]
