Fedora Merge Review: tftp http://cvs.fedora.redhat.com/viewcvs/devel/tftp/ Initial Owner: mbarabas
rpmlint *.src.rpm: W: tftp summary-ended-with-dot The client for the Trivial File Transfer Protocol (TFTP). This should be easy to fix :) W: tftp no-url-tag This should be something like Url: http://foo.redhat.com at minimum or maybe you could point at the upstream home of tftp. W: tftp buildprereq-use tcp_wrappers-devel I'm not really familiar with BuildRequires vs BuildPreRequires here, though it seems like BuildRequires should work fine. W: tftp mixed-use-of-spaces-and-tabs (spaces: line 54, tab: line 55) This isn't hurting anything, though moving install up to the previous line would make this go away. Depends how much you care about rpmlint being whiney. --- Other things I looked over: service logic looks good. xinetd is reloaded on an uninstall or after newinstalls or upgrades. config is noreplaced (good), permissions, good, etc. One potential problem is that the BuildRoot doesn't contain version info (see http://fedoraproject.org/wiki/Packaging/Guidelines)
Is there a reason this bug is closed?
Hi, please take a look at last version in devel.
Martin, looks like you are the new maintainer, so the bug shouldn't be assigned to you, since the bug is assigned to the reviewer. In general the submitter is the Reporter, but not for merge reviews. So, since it is a merge review, you should just be in CC (though it may be nice if you say 'hey I am the current maintainer' when you put yourself in CC)
Hi, I am the new maintainer. Thanks Patrice, for cleaning this up, I don't know why I was assigned to this bug. Could somebody please take the review? The packages in rawhide look good.
This is not the latest version. The current version source timestamp is not kept, so please keep it next time. Some files in %doc are missing, like README (but first update). I suggest adding INSTALL='install -p' on the make install command line to keep timestamps, and also use -p for tftp-xinetd install command. I suggest using %defattr(-,root,root,-) instead of %defattr(-,root,root) I'll certainly review the updated package.
I have updated the package in CVS, please review. Thanks.
Created attachment 292923 [details] correct 2 typos in instal and INSTAL
Looking at the ./configure --help output, it looks like a readline-devel buildrequires is missing?
It looks like it can manage without readline-devel, but it's of course better with it, so I added the dependency, updated in CVS.
Created attachment 293022 [details] use rpm macros instead of hardcoded paths
I proposed a patch to use rpm macros to have config file matching the real installation paths. I'd prefer if you applied it, but even if you don't this is APPROVED.
Fixed in tftp-0.48-1.fc9 Thanks for the review Patrice!