Red Hat Bugzilla – Bug 226489
Merge Review: tftp
Last modified: 2008-01-28 04:42:36 EST
Fedora Merge Review: tftp
Initial Owner: firstname.lastname@example.org
W: tftp summary-ended-with-dot The client for the Trivial File Transfer Protocol
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
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
Fixed in tftp-0.48-1.fc9
Thanks for the review Patrice!