[Bug 457160] Review Request: Zorba - General purpose XQuery processor implemented in C++

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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





--- Comment #4 from Paul F. Kunz <paulfkunz at gmail.com>  2008-08-25 13:01:25 EDT ---
(In reply to comment #1)
> zorba.review.txt:
> Hi, you don't seem to be sponsored, so I can't perform an official review.
> Instead, I can get the review process started with a pre-review. Have you
> requested a fedoraproject account ?

I have an account.   It is pfkeb.   I recently changed my e-mail contact from
paul_kunz at slac.stanford.edu to paulfkunz at gmail.com.


> [??] -devel include shows %name/%name/* . Is this what was intended, why ?
This is the way the upstream installs itself.   I agree it is a little strange.

> 
> [??] are the .TAGFILEs needed by an end user of devel-docs ? Perhaps they are a
> side result of the compile process ?

They come from Doxygen. If the end user wants to link his Doxygen generated
documentation to Zorba's locally then he needs them.

> 
> [??] license is Apache license v2 from web site. extracted upstream source
> mentions "the Apache License" more than 700 times. The short name "APL 2.0" is
> the correct fedora reference.
> 
I put the correct Fedora reference in the spec file.


> [??] NOTICE.txt also provides some license information / history. I haven't
> analysed whether the license would be considered free for Fedora purposes. Have
> each of the authors mentioned 
> 
   I'm not sure what you are suggesting here.

> [??] devel-doc is created as a separate package. It isn't overly large, and
> could potentially be part of the -devel package ? What reasoning caused you to
> split the devel-docs out ?
> 
I've removed devel-docs subpackage.

> [??] spec legible: 
> - could be improved by sticking to a certain coding style within the spec with
> relation to eg 2x blank lines between sections, rather than 0 or 1.
> - the files section has one layout for some subpackages, and different spacing
> for the last ones.
> 
   I've taken you suggestions.   Thanks, it does look better.

> [ ?] might as well fix the spelling of grammer and headerss !
> 
Ran the spell checker on the spec file and fixed the errors.

> [ ?] while individually specifying each %files to include can be done, would it
> be simpler to glob the folders instead (or have you already factored this in) ?
> 
   Good suggestion and done.

> [??] python guidelines suggest placing python_sitelib determination at the top
> of the spec. Any reason to do it elsewhere ?
> 
   its been moved.

> [??] places files directly in the %{python_sitelib}, rather than a module named
> subfolder. Not sure if that is allowed ?
> 
I've seen it done both ways.   Generally, when the package contains multiiple
files and subdirectory is used.   However, zorba has only two files.

> [??] -python doesn't require the base package. Is there a reason why ?
> 
Was oversigth.  Now fixed.

> [??] why put the *.py*, *.gif, *.rb examples in the python/ruby sub packages.
> Would these be more appropriate for the devel-doc package ?
> 
Fixed.
> [??] %build turns on debug output. I don't know whether that is allowed in the
> final package ?
> 
Yes this is allowed.   The debugging symbols are stripped and put in
zorba-debuginfo rpm.

> [ x] rpmlint problems: rpmlint zorba-0.9.21-2.fc9.src.rpm 

I added the attr tag.


> [ x] doesn't build on i386. Is a build require missing ? Perhaps need to try
> one of the methods to help determine build requires at:
> http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRequire

I tried that procedure but it didn't work for me.   Nevertheless I found
additional requires that needed to be added.
> 
> [ x] doesn't own all the dirs it creates:
> %dir %{_datadir}/doc/%{name}-%{version} is the dir only
> the subfolders c, cxx, zorba, python ruby aren't owned {I could be wrong here,
> since it won't build}.

Fixed.
> 
> [ x] unversioned .so must be in the -devel package
> 
moved it.

> 
> [ x] package provides .sos in normal lib dir, but doesn't use the guideline
> must %post/un -p /sbin/ldconfig
> 
Added it.

> [ x] clean rm -rf is commented out. Why has this been done ? I don't think it
> could make it into Fedora like this.
> 
Was commented out for debugging, now put back in.

> [ x] not all %files sections include the %defattr() 
> 
Fixed.
> [ x] main package doc files are not marked as %doc. I assume they aren't
> required for the executable to run.
> 
Correct.

> [ x] LICENSE.txt is included in source, and hence must be included in package,
> but is not marked %doc
> 
Fixed.

> [ x] -python summary line is repeated under description
> 
fixed.
> [ x] -ruby package must indicate the required Ruby ABI version
> 
Done.
> [ x] -ruby library must indicate what it provides with a Provides:
> ruby(LIBRARY) = VERSION
> 
Isn't this done automatically because of the .so file

> [ x] must bump release with each adjustment of the package. This provides
> tracability, and ensures an update path.
> 
Done.

> You don't appear to have begun the fedoraproject account creation process. 

My account if pfkeb

>Note
> the email you use there should be the one used in the changlelog as well. I
> also notice that you are an upstream contributor. What applications are using
> zorba so far ?

Fixed for recent change log entries.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.