Bug 226489

Summary: Merge Review: tftp
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Patrice Dumas <pertusus>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mbarabas, mnagy, pertusus, ruben
Target Milestone: ---Keywords: Reopened
Target Release: ---Flags: pertusus: fedora-review+
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-01-28 09:42:36 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
correct 2 typos in instal and INSTAL
none
use rpm macros instead of hardcoded paths none

Description Nobody's working on this, feel free to take it 2007-01-31 21:10:26 UTC
Fedora Merge Review: tftp

http://cvs.fedora.redhat.com/viewcvs/devel/tftp/
Initial Owner: mbarabas

Comment 1 Michael DeHaan 2007-02-16 23:30:34 UTC
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)




Comment 2 Ruben Kerkhof 2007-06-26 18:48:31 UTC
Is there a reason this bug is closed?

Comment 3 Maros Barabas 2007-07-09 08:57:04 UTC
Hi, please take a look at last version in devel. 

Comment 4 Patrice Dumas 2007-11-28 13:12:45 UTC
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)

Comment 5 Martin Nagy 2007-12-11 11:39:24 UTC
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.

Comment 6 Patrice Dumas 2007-12-17 17:08:52 UTC
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.


Comment 7 Martin Nagy 2008-01-22 13:25:56 UTC
I have updated the package in CVS, please review. Thanks.

Comment 8 Patrice Dumas 2008-01-25 09:38:43 UTC
Created attachment 292923 [details]
correct 2 typos in instal and INSTAL

Comment 9 Patrice Dumas 2008-01-25 09:39:34 UTC
Looking at the ./configure --help output, it looks like a 
readline-devel buildrequires is missing?


Comment 10 Martin Nagy 2008-01-25 10:14:33 UTC
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.

Comment 11 Patrice Dumas 2008-01-25 23:31:58 UTC
Created attachment 293022 [details]
use rpm macros instead of hardcoded paths

Comment 12 Patrice Dumas 2008-01-25 23:33:16 UTC
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.

Comment 13 Martin Nagy 2008-01-28 09:42:36 UTC
Fixed in tftp-0.48-1.fc9
Thanks for the review Patrice!