https://bugzilla.redhat.com/show_bug.cgi?id=1171129
--- Comment #10 from Nikos Mavrogiannopoulos <nmavr...@redhat.com> --- (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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
