Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226489 - Merge Review: tftp
Summary: Merge Review: tftp
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Patrice Dumas
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:10 UTC by Nobody's working on this, feel free to take it
Modified: 2008-01-28 09:42 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-01-28 09:42:36 UTC
pertusus: fedora-review+


Attachments (Terms of Use)
correct 2 typos in instal and INSTAL (deleted)
2008-01-25 09:38 UTC, Patrice Dumas
no flags Details | Diff
use rpm macros instead of hardcoded paths (deleted)
2008-01-25 23:31 UTC, Patrice Dumas
no flags Details | Diff

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@redhat.com

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!


Note You need to log in before you can comment on or make changes to this bug.