[Bug 1171129] Review Request: freeradius-client - Client library and utilities for radius

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

 



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



--- Comment #10 from Nikos Mavrogiannopoulos <nmavrogi@xxxxxxxxxx> ---
(In reply to Mattias Ellert from comment #9)

Thanks for the review. Comments inline.

> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> Since there were some indications in the specfile that this is
> intended for EPEL as well, I ran fedora-review with the -D EPEL5
> flag. If this was not intended, some comments should be modified.

This was unintended. All epel5 leftovers should be removed.

> %prep does "rm -f lib/md5.c", but leaves "lib/md5.h" in place. Shouldn't
> the header file be removed too?

No, that file is needed when used with nettle.

> The URL: tag points to https://github.com/FreeRADIUS/freeradius-client
> On this page the first thing that you see is a link to
> http://freeradius.org/freeradius-client/ which seems to be the upstream
> project website. Would this link be a better choice for the URL tag?

updated.

> The Source0: tag points to
> https://github.com/FreeRADIUS/freeradius-client/archive/release_1_1_7.tar.gz
> The download link of the website points to
> ftp://ftp.freeradius.org/pub/freeradius/freeradius-client-1.1.7.tar.gz
> The content of the tarballs is the same, but which of them is the one
> upstream considers to be their published version?
> Using the one from ftp.freeradius.org you could avoid the
> %global filename release_1_1_7

The link on that site was wrong when I packaged it, but I reported it it is now
fixed. Updated to use that one.

> [!]: If (and only if) the source package includes the text of the license(s)

> [!]: If the package is under multiple licenses, the licensing breakdown must
>      be documented in the spec.

Should be fixed. Added special file with license breakdown.

> [!]: Each %files section contains %defattr if rpm < 4.4

Removed.

> [!]: Package does not generate any conflict.
> [!]: If the package is a rename of another package, proper Obsoletes and

Now it obsoletes that package.

> [!]: All build dependencies are listed in BuildRequires, except for any that
>      are listed in the exceptions section of Packaging Guidelines.
>      What is the purpose of:
>      BuildRequires: autoconf
>      BuildRequires: libtool
>      BuildRequires: automake

Removed. It was leftover when I was building against git.

> [!]: Explicit BuildRoot: tag as required by EPEL5 present.
>      Note: Missing buildroot (required for EPEL5)

No more epel5 compat.

> [!]: Dist tag is present (not strictly required in GL).

Added.

> Generic:
> [!]: Package should not use obsolete m4 macros
>      Note: Some obsoleted macros found, see the attachment.
>      See: https://fedorahosted.org/FedoraReview/wiki/AutoTools
>      This is probably best addressed by forwarding it upstream.
>      See the list below for the obsolete macros found by fedora-review.

I believe that these are autogenerated macros. If you check the open and closed
issues in https://github.com/FreeRADIUS/freeradius-client/issues
I have reported that they should not keep these files forever but auto-generate
them per release. They upstream believes otherwise though.

Anyway. Issues should be addressed.

http://people.redhat.com/nmavrogi/fedora/freeradius-client.spec
http://people.redhat.com/nmavrogi/fedora/freeradius-client-1.1.7-2.src.rpm

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