Bug 591545 - Review Request: apache-commons-net - rename of jakarta-commons-net
Review Request: apache-commons-net - rename of jakarta-commons-net
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mat Booth
Fedora Extras Quality Assurance
:
Depends On:
Blocks: JakartaCommonsRename
  Show dependency treegraph
 
Reported: 2010-05-12 10:28 EDT by Stanislav Ochotnicky
Modified: 2010-05-19 15:35 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-05-19 15:35:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mat.booth: fedora‑review+
dennis: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Stanislav Ochotnicky 2010-05-12 10:28:40 EDT
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-12 23:07:11 EDT
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 10:33:55 EDT
I can take this one.
Comment 3 Stanislav Ochotnicky 2010-05-14 04:17:03 EDT
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 08:12:45 EDT
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 03:15:47 EDT
(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 14:28:12 EDT
CVS Done
Comment 8 Alexander Kurtakov 2010-05-19 15:35:07 EDT
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.