[Bug 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #21 from Fedora Update System  ---
camotics-1.1.1-6.el7 has been pushed to the Fedora EPEL 7 stable repository. If
problems still persist, please make note of it in this bug report.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #20 from Fedora Update System  ---
camotics-1.1.1-7.fc26 has been pushed to the Fedora 26 stable repository. If
problems still persist, please make note of it in this bug report.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #19 from Fedora Update System  ---
camotics-1.1.1-7.fc25 has been pushed to the Fedora 25 stable repository. If
problems still persist, please make note of it in this bug report.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA
Last Closed||2017-07-15 14:49:48



--- Comment #18 from Fedora Update System  ---
camotics-1.1.1-7.fc24 has been pushed to the Fedora 24 stable repository. If
problems still persist, please make note of it in this bug report.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1432983
Bug 1432983 depends on bug 1457949, which changed state.

Bug 1457949 Summary: Review Request: libdxflib - A C++ library for reading and 
writing DXF files
https://bugzilla.redhat.com/show_bug.cgi?id=1457949

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA



-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #17 from Fedora Update System  ---
camotics-1.1.1-7.fc26 has been pushed to the Fedora 26 testing repository. If
problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-045ae0bbc9

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #16 from Fedora Update System  ---
camotics-1.1.1-7.fc24 has been pushed to the Fedora 24 testing repository. If
problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-25c9eb1f73

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #15 from Fedora Update System  ---
camotics-1.1.1-7.fc25 has been pushed to the Fedora 25 testing repository. If
problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-b10e857c05

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #14 from Fedora Update System  ---
camotics-1.1.1-6.el7 has been pushed to the Fedora EPEL 7 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-8e5487b2e9

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #13 from Fedora Update System  ---
camotics-1.1.1-6.el7 has been submitted as an update to Fedora EPEL 7.
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-8e5487b2e9

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #12 from Fedora Update System  ---
camotics-1.1.1-7.fc26 has been submitted as an update to Fedora 26.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-045ae0bbc9

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ASSIGNED|MODIFIED



-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #11 from Gwyn Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/camotics

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

Lubomir Rintel  changed:

   What|Removed |Added

  Flags|fedora-review?  |fedora-review+



--- Comment #10 from Lubomir Rintel  ---
Thanks. This looks cool now!

APPROVED

I've also sponsored you into the packager group. You should have gotten the
welcome e-mail by now. You can go ahead and request a repository for the
package in pkgdb, import and build it. Thank you!

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

srakitnican  changed:

   What|Removed |Added

 Depends On||1457949



--- Comment #9 from srakitnican  ---
I believe I've got everything now and some more! (or less)
SPEC:
http://copr-dist-git.fedorainfracloud.org/cgit/srakitnican/default/camotics.git/plain/camotics.spec?id=d0a5a1e8a194d317b5687b91d8ead7f10b84
SRPM:
https://copr-be.cloud.fedoraproject.org/results/srakitnican/default/epel-7-x86_64/00560685-camotics/camotics-1.1.1-5.el7.centos.src.rpm

camotics-1.1.1-5


Changes from 1.1.1-3:

* Use system dxflib (depends on bug 1457949)
* Boost is bundled (change in upstream)
* Unbundled re2
* Build using c++11 compiler standard for re2
* Pass the option to build system to exclude bzip2, expat, re2, sqlite3 and
zlib
* Corrected License tag according to bundled libraries
* Reused common compiler flags and build options from global variables
* Included AppStream Metadata
* Added missing bzip2-devel build requirement


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1457949
[Bug 1457949] Review Request: libdxflib - A C++ library for reading and
writing DXF files
-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #8 from srakitnican  ---
The requested change in dxflib was partially accepted upstream, so I went ahead
and built a package with required changes: bug 1457949

I have a working camotics package with such a change already.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #7 from srakitnican  ---
camotics-1.1.1-3

SPEC URL:
http://copr-dist-git.fedorainfracloud.org/cgit/srakitnican/default/camotics.git/plain/camotics.spec?id=29ed27f71d3772f02ae02245c978d23a6d54bcdc
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/srakitnican/default/fedora-25-x86_64/00557191-camotics/camotics-1.1.1-3.fc25.x86_64.rpm

Changes from 1.1.1-2:

* Use Cairo and GLEW from system
* Updated cbang that includes additional fixes 


Joseph Coffland implemented possibility to use Cairo, GLEW and dxflib as system
libraries if present. I've backported Cairo and GLEW patches to version 1.1.1.
Although CAMotics can use dxflib from system, Joseph pointed out that there is
a feature missing in dxflib that CAMotics require. Joseph also submited pull
request upstream [1].

Until this is sorted out we can't make use of dxflib as a system library.


Interestingly unbundling Cairo and GLEW seems to fix coredump issue, so now
CAMotics works as a debug build.


[1] https://github.com/qcad/qcad/pull/15

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #6 from srakitnican  ---
Spec URL:
http://copr-dist-git.fedorainfracloud.org/cgit/srakitnican/default/camotics.git/plain/camotics.spec?id=0166e50ff661e785a5ed7e23c276ee7875b7e81a
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/srakitnican/default/fedora-25-x86_64/00556754-camotics/camotics-1.1.1-2.fc25.src.rpm

Changes from the last version:

* mariadb-devel seems to not be required so I've dropped it
* qt-devel and openssl-devel as pkgconfig modules
* Removed strip step
* Debug build
* Patch to fix misleading-indentation error that shows up with particular flags
* Updated cbang, fixes compilation error with optflags
* Commented unusual procedures
* Added Provides bundle(cbang)
* Added gcc-c++ build requirement
* Corrected license tag partially
* Fixed line-endings and removed executable bit from datadir files

Still left to do:

* Debug build possibly does not work (compiled executable aborts with a
coredump)
* AppStream metadata
* Possibly unbundle Cairo, GLEW and dxflib
* License tag according to bundled libraries


(In reply to Lubomir Rintel from comment #4)
> (In reply to srakitnican from comment #3)
> > With debug flag I ran into an issue where compiled binary immediately
> > crashed without starting up. Also compiled binaries file sizes did not look
> > right, e.g. smaller then upstream. So debug_package is here in order for
> > rpmbuild to not error out on missing debuginfo.
> 
> Well, this needs to be fixed first.
> 
> I could help with that, but am not able to reproduce the issue. Please share
> as much information as you can -- what version and architecture are you
> building on?  Can you reproduce the bug with a mock build?

Yes, I am using only mock.

In this version I am trying to make a debug build, so I've removed everything
that would otherwise prevent this from happening. Binaries size looks correct,
but produced binaries (stripped or unstripped) does not work for me. In order
to reproduce failed build I can simply add debug=1 option to scons in all
stages.

Without scons debug=1 flag, binaries seems to get "stripped" by build system
before rpm gets a chance to do anything.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #5 from Lubomir Rintel  ---
Rest of the review:

* Package named correctly
* Packaging the latest version
* License good for Fedora (see below for remarks about the license tag)
* License text included
* Filelist sane
* Requires/Provides sane
* Scriptlets look good

6.) Please unbundle cairo, GLEW and perhaps dxflib (ideally ask upstream to
allow building against system copies). See the wiki reference above for
explanation why bundling is bad.

7.) Use the correct compiler flags. E.g. change

  scons ccflags="%{?fedora:-I%{_includedir}/v8-3.14/}" 

to

  scons ccflags="%{?fedora:-I%{_includedir}/v8-3.14/} %{optflags}"

wherever appropriate.

8.) The license tag is incorrect:

  License:LGPLv2

It looks like the camotics source is GPLv2+. (Some of the bundled files use
different licenses -- if it's not possible to unbundle them then they should be
included in the License tag).

9.) rpmlint is unhappy

camotics.x86_64: W: wrong-file-end-of-line-encoding
/usr/share/doc/camotics/examples/tiger/tiger.nc
camotics.x86_64: W: spurious-executable-perm
/usr/share/doc/camotics/examples/camotics/camotics.svg
camotics.x86_64: W: wrong-file-end-of-line-encoding
/usr/share/doc/camotics/examples/cameo/cameo.nc
camotics.x86_64: E: script-without-shebang
/usr/share/camotics/tpl_lib/clipper/clipper.js
camotics.x86_64: W: wrong-file-end-of-line-encoding
/usr/share/doc/camotics/examples/stl-test/V1.stl

Please at least remove the executable bits; ideally also recode the DOS line
endings in examples.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

Lubomir Rintel  changed:

   What|Removed |Added

 CC||lrin...@redhat.com



--- Comment #4 from Lubomir Rintel  ---
(In reply to srakitnican from comment #3)
> (In reply to Lubomir Rintel from comment #1)
> > 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
> 
> Well, initially I had it as a separate package, but I've ran into all sort
> of problems because it is not really intended to be packaged separately, as
> the author itself told me. For example CAMotics is using configuration files
> from cbang for scons and build dependencies checking is done as a one unit.
> Also I have found cbang compiles mostly into static library files. If you
> still think that should be packaged separately, I can do that.

That is fair enough. However, please add a comment explaining why do you bundle
cbang and add a Provides: bundled(cbang) as described in the wiki paragraph
linked to above.

> > 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.
> 
> With debug flag I ran into an issue where compiled binary immediately
> crashed without starting up. Also compiled binaries file sizes did not look
> right, e.g. smaller then upstream. So debug_package is here in order for
> rpmbuild to not error out on missing debuginfo.

Well, this needs to be fixed first.

I could help with that, but am not able to reproduce the issue. Please share as
much information as you can -- what version and architecture are you building
on?  Can you reproduce the bug with a mock build?

> > 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.
> > 
> 
> Submited and included, will make a note in spec file.
> https://github.com/CauldronDevelopmentLLC/CAMotics/issues/211

Thank you.

> > 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}
> 
> cbang does not work with newer version of v8 which Fedora defaults to. I
> guess conditional is not strictly required, but I though it would be more
> straightforward to mark what exactly needs it because RHEL still uses 3.14
> as default.

That is okay. But please add a comment explaining why are you doing that.

> > 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.
> 
> Thanks!

You're welcome.

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #3 from srakitnican  ---
(In reply to Lubomir Rintel from comment #1)
> 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

Well, initially I had it as a separate package, but I've ran into all sort of
problems because it is not really intended to be packaged separately, as the
author itself told me. For example CAMotics is using configuration files from
cbang for scons and build dependencies checking is done as a one unit. Also I
have found cbang compiles mostly into static library files. If you still think
that should be packaged separately, I can do that.


> 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.

With debug flag I ran into an issue where compiled binary immediately crashed
without starting up. Also compiled binaries file sizes did not look right, e.g.
smaller then upstream. So debug_package is here in order for rpmbuild to not
error out on missing debuginfo.


> 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.
> 

Submited and included, will make a note in spec file.
https://github.com/CauldronDevelopmentLLC/CAMotics/issues/211

> 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}

cbang does not work with newer version of v8 which Fedora defaults to. I guess
conditional is not strictly required, but I though it would be more
straightforward to mark what exactly needs it because RHEL still uses 3.14 as
default.


> 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.

Thanks!

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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



--- Comment #2 from Lubomir Rintel  ---
Also, please ask upstream to consider shipping AppStream metadata. That way the
package would be installable via app stores such as GNOME Software.
https://fedoraproject.org/wiki/Packaging:AppData

-- 
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 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

Lubomir Rintel  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 CC||lkund...@v3.sk
   Assignee|nob...@fedoraproject.org|lkund...@v3.sk
  Flags||fedora-review?



--- Comment #1 from Lubomir Rintel  ---
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 -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1432983] Review Request: camotics - Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator

[bugzilla]

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

srakitnican  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