Bug 591545 - Review Request: apache-commons-net - rename of jakarta-commons-net
Summary: Review Request: apache-commons-net - rename of jakarta-commons-net
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mat Booth
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: JakartaCommonsRename
TreeView+ depends on / blocked
 
Reported: 2010-05-12 14:28 UTC by Stanislav Ochotnicky
Modified: 2010-05-19 19:35 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2010-05-19 19:35:07 UTC
Type: ---
Embargoed:
mat.booth: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Stanislav Ochotnicky 2010-05-12 14:28:40 UTC
Note that this is a re-review of jakarta-commons-net package (changed name upstream)

Spec URL: http://sochotni.fedorapeople.org/apache-commons-net.spec
SRPM URL: http://sochotni.fedorapeople.org/apache-commons-net-2.0-1.fc12.src.rpm

Description:
This is an Internet protocol suite Java library originally developed by
ORO, Inc.  This version supports Finger, Whois, TFTP, Telnet, POP3, FTP,
NNTP, SMTP, and some miscellaneous protocols like Time and Echo as well
as BSD R command support. The purpose of the library is to provide
fundamental protocol access, not higher-level abstractions.

Comment 1 Chen Lei 2010-05-13 03:07:11 UTC
When possible, we should avoid of using Epoch, it may interfere some other packages which depends on particular version of apache-commons-net.


In this package, we can avoid of using epoch in the following two ways:

Release:        3%{?dist}
Provides:       jakarta-%{short_name} = %{version}-%{release}
Obsoletes:      jakarta-%{short_name} < 2.0-3





See 
http://fedoraproject.org/wiki/Package_Renaming_Process

http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Renaming.2Freplacing_existing_packages

Comment 2 Mat Booth 2010-05-13 14:33:55 UTC
I can take this one.

Comment 3 Stanislav Ochotnicky 2010-05-14 08:17:03 UTC
Please hold the review until I upload new spec/srpm. Chen is right and there will be bigger modifications to the spec because of this change.

Comment 5 Mat Booth 2010-05-15 12:12:45 UTC
I note that this is a re-review due to a package rename.

When you say Chen is right, does that mean you intended to drop the use of epoch in your provides/obsoletes? Is there any point in specifying an epoch of zero?

Apart from that, there is really nothing else wrong with the package. The rpmlint report only has false positives, the rest of the package is to the guidelines. You've clearly been taking notes from the other apache-commons-* reviews, I like easy reviews. :-)

On the condition that you just clarify your position on the use of epoch as mentioned above, this package is:

APPROVED!



On a side note for future reference, did you know that your two calls to install on lines 71/72 and again on lines 83/84 can be combined into a single call? For example, these two lines:

install -d -m 755 foo_dir
install -p -m 644 bar_file foo_dir/bar_file

Are equivalent to this one line:

install -pD -T -m 644 bar_file foo_dir/bar_file

Comment 6 Stanislav Ochotnicky 2010-05-17 07:15:47 UTC
(In reply to comment #5)
> When you say Chen is right, does that mean you intended to drop the use of
> epoch in your provides/obsoletes? Is there any point in specifying an epoch of
> zero?

I meant that packaging guidelines state that we should avoid using Epoch if possible. This was possible with this rename because it was enough to specify epoch in provides/obosletes, no need to actually specify one for this package. It was there from way back, probably when no guidelines on Epoch use existed.

Current guidelines state (2nd link from Chen):
If the provided package had an Epoch set, it must be preserved in both the Provides and Obsoletes. It may and should be removed from the actual new package. 


> On a side note for future reference, did you know that your two calls to
> install on lines 71/72 and again on lines 83/84 can be combined into a single
> call? For example, these two lines:
> 
> install -d -m 755 foo_dir
> install -p -m 644 bar_file foo_dir/bar_file
> 
> Are equivalent to this one line:
> 
> install -pD -T -m 644 bar_file foo_dir/bar_file    

Thanks, I'll keep it in mind :-) I suppose 2 commands were originally mkdir && cp and I just converted them to install.


New Package CVS Request
=======================
Package Name: apache-commons-net
Short Description: Internet protocol suite Java library
Owners: sochotni
Branches: 
InitialCC:

Comment 7 Dennis Gilmore 2010-05-18 18:28:12 UTC
CVS Done

Comment 8 Alexander Kurtakov 2010-05-19 19:35:07 UTC
Koji build:
http://koji.fedoraproject.org/koji/buildinfo?buildID=174135


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