Bug 448310

- Summary: A partial PureBasic to C++ cross-compiler
? Generation commands should use svn export instead
- The ruby BR is covered by ruby-racc
? "C++" case in %description
? "make: bzr: Command not found" - not a blocker, but it is ugly (I thought it
was pulled from svn...)
- I don't like that the lostlaby patches are in here instead of the
lostlabyrinth SRPM
. Noted that 2 lostlabyrinth-specific executables are included
??? HOLY CRAP! Is that really all the font data from Vera.ttf included in the
script?!
? No license reference in many of the source files
? Tests aren't run in %check

(In reply to comment #1)
> - Summary: A partial PureBasic to C++ cross-compiler
> ? Generation commands should use svn export instead

I don't use svn that often, but indeed export is much better in this case

> - The ruby BR is covered by ruby-racc

Only implicit not (through /usr/bin/ruby and ruby(abi) = 1.8) not explicit and
the ruby guidelines:
http://fedoraproject.org/wiki/Packaging/Ruby
say:
"Ruby packages must require ruby at build time with a BuildRequires: ruby"

> ? "C++" case in %description

Fixed

> ? "make: bzr: Command not found" - not a blocker, but it is ugly (I thought it
> was pulled from svn...)

I know, I guess the elice author uses bzr for its own internal version tracker
and then from time to time submits his work to Lost Labyrinths svn. Nothing I
can do here really.

> - I don't like that the lostlaby patches are in here instead of the
> lostlabyrinth SRPM

Oh, those are no longer needed, they have been integrated into Lost Labyrinth
2.9.2 -> dropped.

> . Noted that 2 lostlabyrinth-specific executables are included

Yip, as said in the description, this really is only meant for building Lost
Labyrinth

> ??? HOLY CRAP! Is that really all the font data from Vera.ttf included in the
> script?!

Erm, yes it would seem so, I will contact upstream about this asking them to
just use a file instead.

> ? No license reference in many of the source files

The licensing for all relevant files (including a file list) is explained in the
file titled COPYING (no this is not just a copy of the GPL).

> ? Tests aren't run in %check

Good catch! Fixed.

Here is a new version with most issues fixed:
Spec URL: http://people.atrpms.net/~hdegoede/elice.spec
SRPM URL: http://people.atrpms.net/~hdegoede/elice-0.0-0.2.svn257.fc9.src.rpm

Erm, ping?

Sorry, I missed the links at the bottom.

- Release should be 0%{?dist}.2.svn%{svn_revision}
? make check runs, but I'm not convinced it does anything

Other than those, APPROVED.

(In reply to comment #4)
> Sorry, I missed the links at the bottom.
> 
NP.

> - Release should be 0%{?dist}.2.svn%{svn_revision}

Erm, where did you get that from ??
[hans@localhost ~]$ rpm -qa | grep svn
qimageblitz-0.0.4-0.4.svn706674.fc9.x86_64
kdnssd-avahi-devel-0.1.3-0.6.20080116svn.fc9.x86_64
NetworkManager-vpnc-0.7.0-0.7.7.svn3502.fc9.x86_64
libflashsupport-000-0.3.svn20070904.i386
libnetfilter_conntrack-0.0.89-0.1.svn7356.fc9.x86_64
<much more snipped>

[hans@localhost ~]$ rpm -qa | grep cvs
mdbtools-devel-0.6-0.4.cvs20051109.fc9.x86_64
xmldb-api-sdk-0.1-0.2.20011111cvs.1jpp.2.fc9.x86_64
xu4-1.1-0.4.cvs20070510.fc9.x86_64
<much more snipped>

Also from:
http://fedoraproject.org/wiki/Packaging/DistTag

"Basically, follow the ["Packaging/NamingGuidelines"] for how to set the value
for Release, then append %{?dist} to the end." Notice the "append %{?dist} to
the end."

> ? make check runs, but I'm not convinced it does anything

It does I wasn't convinced either and I checked, its just very quiet when
everything goes ok.

I've just read a mail from upstream that they are making available tarballs now,
so here is a new version which uses a tarbal instead of an svn snapshot:

http://people.atrpms.net/~hdegoede/elice.spec
http://people.atrpms.net/~hdegoede/elice-0.258-1.fc10.src.rpm

(In reply to comment #5)
> Also from:
> http://fedoraproject.org/wiki/Packaging/DistTag
> 
> "Basically, follow the ["Packaging/NamingGuidelines"] for how to set the value
> for Release, then append %{?dist} to the end." Notice the "append %{?dist} to
> the end."

Interesting. It could be interpreted as conflicting with
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Minor_release_bumps_for_old_branches
then.

Oh well, a moot point now.

APPROVED

New Package CVS Request
=======================
Package Name:      elice
Short Description: Elice is a PureBasic to c++ translator / compiler
Owners:            jwrdegoede
Branches:          F-8 F-9
InitialCC:
Cvsextras Commits: yes

cvs done.

Imported and build. For F-8 and F-9 Í've requested tagging into the
buildroot-verride tag, once lostlabyrinth is build I will push an elice update
together with lostlaby.

p.s.

Thanks for the review!