[Bug 1100409] Review Request: libixion - a general purpose formula parser & interpreter library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Björn "besser82" Esser <bjoern.esser@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bjoern.esser@xxxxxxxxx



--- Comment #2 from Björn "besser82" Esser <bjoern.esser@xxxxxxxxx> ---
Quite a good job, Florian!  Things I don't explicitly mention here are fine.

(In reply to Florian "der-flo" Lehner from comment #1)
> This is an *INFORMAL* package-review
> 
> [!] koij-build is missing

---> That's no real neccessity…  But mostly welocmed by reviewers, so they
     have a proof the pkg does *at least* build successful before starting
     the review.


> [!]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses found:
>      "GPL (v2 or later)", "Unknown or generated", "*No copyright* MPL
> (v2.0)".
>      8 files have unknown license.
>      ---> Note about license is missing in all files in /libixion-0.7.0/bin/

---> That's not the real point about the license check.  ;)  There are a
     lot of packages with files, that don't have any license-header in them.
     The real point is to e.g. get some evidence about bundled libs and such.

     As long as the license-tag in spec-file matches the actual license from
     COPYING / LICENSE and the corresponding source-files everything is fine.

     There are some cases, when a different license doesn't need to be inside
     license-tag.  For this one it is the file called `ltmain.sh`, which is
     part of the autotools buildsys and doesn't get installed by the resulting
     packages.  btw.  the files inside that ./bin/ dir are used for invoking
     the testsuite during %check.  The also don't get installed by the
     resulting binary-pkgs.


> [!]: License file installed when any subpackage combination is installed.
>      ---> COPYING is not installed for subpackages

---> COPYING is in main-pkg, which is fine.  The other subpackages do have
     a requirement for the mainpkg: `%{name}%{?_isa} = %{version}-%{release}`
     So the file gets installed with the mainpkg even when one installes the
     devel or tools pkg.


> [!]: Package consistently uses macros (instead of hard-coded directory
> names).
>      ---> use %{_sbindir} instead of /sbin/ 

---> since /sbin/ is part of the FHS and used only in %post{in,un} everthing
     is fine about that.  The way that sniplet is used is even documentet
     this way in [1].


> [x]: Package complies to the Packaging Guidelines

---> If you find issues in the package, you usually wouldn't check
     this `PASS`.  ^^


> [x]: Package should compile and build into binary rpms on all supported
>      architectures.

---> That's where the koji-build comes in handy.  Since the reported didn't
     do one, did you to assure yourself?


> [!]: Packages should try to preserve timestamps of original installed files.
>      ---> add -p to the install commands

---> One may do this, but in this special case I don't see any *strong*
     reason for it.  The man-pages, which are installed by the invocation of
     `install`, in question are generated on the fly during %build, using
     `help2man`, so the timestamp won't be the same during two different
     builds.  If those would have been static files with a fixed timestamp,
     I'd strongly agree upon using the `-p`-switch on install.


[1]  http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Shared_libraries

-- 
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@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]