[Bug 1917948] Review Request: iceauth - X11 Inter-Client Exchange authority file utility

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

 



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

David Cantrell <dcantrell@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(ajax@xxxxxxxxxx)



--- Comment #2 from David Cantrell <dcantrell@xxxxxxxxxx> ---
(In reply to David Cantrell from comment #1)
> [ ]: Provides: bundled(gnulib) in place as required.
>      Note: Sources not installed

It does not.

> [ ]: Package does not contain kernel modules.

Not even Mr. Redenbacher could find one.

> [ ]: Package contains no static executables.

That is a correct and true statement.

> [ ]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.

It is.  MIT.

> [ ]: If (and only if) the source package includes the text of the
>      license(s) in its own file, then that file, containing the text of the
>      license(s) for the package is included in %license.

FAIL.  Package needs:

%license COPYING

> [ ]: License field in the package spec file matches the actual license.
>      Note: There is no build directory. Running licensecheck on vanilla
>      upstream sources. No licenses found. Please check the source files for
>      licenses manually.

It does.

> [ ]: License file installed when any subpackage combination is installed.

FAIL.  Package lacks "%license COPYING"

> [ ]: %build honors applicable compiler flags or justifies otherwise.

It does.

> [ ]: Package contains no bundled libraries without FPC exception.

Correct.

> [ ]: Changelog in prescribed format.

Very much so.

> [ ]: Sources contain only permissible code or content.

Only the most permissible.

> [ ]: Package contains desktop file if it is a GUI application.

N/A

> [ ]: Development files must be in a -devel package

N/A

> [ ]: Package uses nothing in %doc for runtime.

Correct.

> [ ]: Package consistently uses macros (instead of hard-coded directory
>      names).

Yes.

> [ ]: Package is named according to the Package Naming Guidelines.

Yes.

> [ ]: Package does not generate any conflict.

This is subjective, right?  Unfortunately it carries the name "iceauth" and one
not familiar with this software might think it's related to Immigration and
Customs Enforcement in which case the executable name "iceauth" is even more of
a bold statement.  However, it being part of Xorg I think it's clear that it is
part of the Inter-Client Exchange.

I am going to say that it does not generate conflict though the name could, in
theory, be pointed to as problem language.

> [ ]: Package obeys FHS, except libexecdir and /usr/target.

It does.

> [ ]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.

N/A

> [ ]: Requires correct, justified where necessary.

Yes.

> [ ]: Spec file is legible and written in American English.

You bet!

> [ ]: Package contains systemd file(s) if in need.

N/A

> [ ]: Useful -debuginfo package or justification otherwise.

Sure.

> [ ]: Package is not known to require an ExcludeArch tag.

I know it's not required and know the package does not carry the unknown tag.

> [ ]: Package complies to the Packaging Guidelines

Yep.

> ===== SHOULD items =====
> 
> Generic:
> [ ]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.

License is there, the package needs the %license directive.

> [ ]: Final provides and requires are sane (see attachments).

They are.

> [ ]: Package functions as described.

Yes.

> [ ]: Latest version is packaged.

I assume so.

> [ ]: Package does not include license text files separate from upstream.

Correct.

> [ ]: Sources are verified with gpgverify first in %prep if upstream
>      publishes signatures.
>      Note: gpgverify is not used.

N/A

> [ ]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.

N/A

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

It does.

> [ ]: %check is present and all tests pass.

N/A

> [ ]: Packages should try to preserve timestamps of original installed
>      files.

Sure.

> [x]: Reviewer should test that the package builds in mock.

Done.


Other notes:
------------

1) There's an empty %doc directive.  Shouldn't that be "%doc ChangeLog INSTALL
README" or something along those lines?

2) There is no %license line for COPYING (noted above).

3) Are the BuildRequires wrapped in "%if 0/%endif" required for anything?  Can
they be removed?


-- 
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
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux