Bug 555941 - Review Request: eclipse-manpage - Man page viewer
Summary: Review Request: eclipse-manpage - Man page viewer
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:
TreeView+ depends on / blocked
 
Reported: 2010-01-15 23:06 UTC by Alexander Kurtakov
Modified: 2010-01-20 13:22 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2010-01-20 13:21:34 UTC
Type: ---
Embargoed:
mat.booth: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Alexander Kurtakov 2010-01-15 23:06:18 UTC
Spec URL: http://akurtakov.fedorapeople.org/eclipse-manpage.spec
SRPM URL: http://akurtakov.fedorapeople.org/eclipse-manpage-0.0.1-0.svn24060.fc12.src.rpm
Description: Plugin providing common interface for displaying a man page in a view or fetching it's content for embedded display purposes (e.g hover help).

Comment 1 Mat Booth 2010-01-16 00:46:50 UTC
A review is forthcoming. :-)

Comment 2 Mat Booth 2010-01-18 10:37:58 UTC
Sorry for the delay, eventful weekend...


Nitpick 1:

SPEC file must be in American English: There's a tiny grammatical error in the package description; there should be no apostrophe in the possessive form of "it".

Nitpick 2:

I know it isn't the case for this package, but I think it's good practice to check for class files and jars in the source for Java packages, as per the Java package template [1] or the bit of script from the Eclipse plug-in guidelines. [2]

Nitpick 3:

Please use %global instead of %defined. [3] If these lines were generated by RPMStubby, we should probably file a bug there too.


If you are happy to address these three things, then I say this package is APPROVED.


[1] http://fedoraproject.org/wiki/Packaging:Java#ant
[2] http://fedoraproject.org/wiki/Packaging:EclipsePlugins#Pre-built_binaries
[3] http://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

Comment 3 Alexander Kurtakov 2010-01-18 19:19:45 UTC
(In reply to comment #2)
> Sorry for the delay, eventful weekend...
> 
> 
> Nitpick 1:
> 
> SPEC file must be in American English: There's a tiny grammatical error in the
> package description; there should be no apostrophe in the possessive form of
> "it".
Fixed.

> 
> Nitpick 2:
> 
> I know it isn't the case for this package, but I think it's good practice to
> check for class files and jars in the source for Java packages, as per the Java
> package template [1] or the bit of script from the Eclipse plug-in guidelines.
> [2]
Fixed. Though as an upstream developer I don't think this is needed.

> 
> Nitpick 3:
> 
> Please use %global instead of %defined. [3] If these lines were generated by
> RPMStubby, we should probably file a bug there too.
Fixed and rpmstubby bug opened.

> 
> 
> If you are happy to address these three things, then I say this package is
> APPROVED.
> 
> 
> [1] http://fedoraproject.org/wiki/Packaging:Java#ant
> [2] http://fedoraproject.org/wiki/Packaging:EclipsePlugins#Pre-built_binaries
> [3]
> http://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define    

New sources:

Spec URL: http://akurtakov.fedorapeople.org/eclipse-manpage.spec
SRPM URL:
http://akurtakov.fedorapeople.org/eclipse-manpage-0.0.1-0.svn24060.1.fc12.src.rpm

Comment 4 Mat Booth 2010-01-18 19:49:29 UTC
APPROVED

Comment 5 Alexander Kurtakov 2010-01-18 19:53:22 UTC
New Package CVS Request
=======================
Package Name: eclipse-manpage
Short Description: Man page viewer
Owners: akurtakov
Branches: 
InitialCC:

Comment 6 Jason Tibbitts 2010-01-19 19:24:43 UTC
CVS done (by process-cvs-requests.py).

Comment 7 Alexander Kurtakov 2010-01-20 13:21:34 UTC
Build in rawhide.
http://koji.fedoraproject.org/koji/buildinfo?buildID=151950

Comment 8 Alexander Kurtakov 2010-01-20 13:21:47 UTC
Build in rawhide.
http://koji.fedoraproject.org/koji/buildinfo?buildID=151950


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