[Bug 736062] Review Request: ppc64-diag - Linux for Power Platform Diagnostics

bugzilla at redhat.com bugzilla at redhat.com
Wed Sep 7 13:11:17 UTC 2011


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=736062

Nils Philippsen <nphilipp at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #3 from Nils Philippsen <nphilipp at redhat.com> 2011-09-07 09:11:15 EDT ---
(In reply to comment #2)
> - servicelog is the name of a package, we can't change that easily to
> service-log. But I've changed the description to 'service log'

OK. Rpmlint now only complains about the missing upstream tarball, "config" and
that a number of binaries don't have man pages (maybe pass that on to
upstream):

    nils at gibraltar:~/devel/reviews/fedora/ppc64-diag> rpmlint
ppc64-diag-2.4.2-2.fc16.ppc64.rpm
    ppc64-diag.ppc64: W: spelling-error %description -l en_US config -> con
fig, con-fig, configure
    ppc64-diag.ppc64: W: no-manual-page-for-binary convert_dt_node_props
    ppc64-diag.ppc64: W: no-manual-page-for-binary rtas_errd
    ppc64-diag.ppc64: W: no-manual-page-for-binary add_regex
    ppc64-diag.ppc64: W: no-manual-page-for-binary extract_platdump
    ppc64-diag.ppc64: W: no-manual-page-for-binary diag_encl
    1 packages and 0 specfiles checked; 0 errors, 6 warnings.

> - I'd like to keep the pointer to the config file in the description as long as
> the man pages are missing that describe where the daemon can be configured.

OK

> - replaced systemd path with %{_unitdir}

I guess this got pulled in via another dependency, but to be safe systemd-units
should be required directly for building (forgot about that above, sorry):

    BuildRequires: systemd-units

> - added dist tag
>
> - license is EPL, http://sourceforge.net/projects/linux-diag/ is a page for
> several other packages, too. Those have these other licenses.

OK

> - the tarball somehow got deleted from sourceforge, I've requested that it gets
> uploaded again in https://bugzilla.redhat.com/show_bug.cgi?id=731419#c6

Do you have an idea how long it will take until the file is available again?

> - added Type=forking to systemd service file
>
> - added smp flags
>
> - package now owns %{_datadir}/ppc64-diag
>
> - all references for /ppc64-diag/ subdirectories got replaced by %{name}
> - fixed usage of %_libexecdir and %_sbindir
>
> - changed file requirements to package requirements
>
> - removed chkconfig dependency, that was for the sysv-initscript
>
> - perl dependency gets added automatically:
> # rpm -qp --requires ppc64-diag-2.4.2-2.fc16.ppc64.rpm| grep perl
> /usr/bin/perl
> perl(Getopt::Long)

OK

> The permissions after a make install are messed up, that's why each file has a
> %attr:
> ppc64-diag.ppc64: E: non-standard-executable-perm
> /usr/share/ppc64-diag/message_catalog/e1000e 0744L
[...]
> ppc64-diag.ppc64: E: non-standard-executable-perm
> /usr/share/ppc64-diag/ppc64_diag_migrate 0744L

OK, please ask upstream to fix that.

I approve this package, just add that systemd-units build dep and ensure that
upstream makes the tarball available again soon.

You can now continue with step 8 for contributors in the package review
process: http://fedoraproject.org/wiki/Package_Review_Process#Contributor

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



More information about the package-review mailing list