This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 470792

Summary: Review Request: eclipse-shelled - Shell script editor plugin for Eclipse
Product: [Fedora] Fedora Reporter: Alexander Kurtakov <akurtako>
Component: Package ReviewAssignee: Orcan Ogetbil <oget.fedora>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, oget.fedora
Target Milestone: ---Flags: oget.fedora: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-12-03 12:35:03 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Alexander Kurtakov 2008-11-10 05:32:50 EST
Spec URL:
http://akurtakov.fedorapeople.org/eclipse-shelled.spec
SRPM URL:
http://akurtakov.fedorapeople.org/eclipse-shelled-1.0.3-1.fc9.src.rpm

Description: ShellEd is a shell script editor for Eclipse. 
The great benefit of this plugin is the integration of man page information
 for content assist and text hover.
Comment 1 Orcan Ogetbil 2008-11-25 01:46:21 EST
I made a quick check. There is this issue that needs to be corrected before we can do a full review:

* Source0 must be given in full URL from upstream's website.


A few other things that I caught were:

* For the Group tag pick something from
   /usr/share/doc/rpm-*/GROUPS
Development/Tools or Development/Languages maybe?

* %eclipse_base must be
   %define eclipse_base %{_datadir}/eclipse
Check: http://fedoraproject.org/wiki/Packaging/EclipsePlugins
Otherwise the package will not build.

* No %doc files? Check the source thoroughly. Please list every applicable file in %doc.

* Use CPL for the license

* There shouldn't be both spaces and tabs in the same SPEC file. Please use one or the other. rpmlint is a good application which will warn you about such things and many other issues.
Comment 2 Alexander Kurtakov 2008-11-25 04:40:34 EST
Updated with the comments 

Spec URL:
http://akurtakov.fedorapeople.org/eclipse-shelled.spec
SRPM URL:
http://akurtakov.fedorapeople.org/eclipse-shelled-1.0.3-1.fc9.src.rpm


(In reply to comment #1)
> I made a quick check. There is this issue that needs to be corrected before we
> can do a full review:
> 
> * Source0 must be given in full URL from upstream's website.
Done: Fetch script added.

> 
> 
> A few other things that I caught were:
> 
> * For the Group tag pick something from
>    /usr/share/doc/rpm-*/GROUPS
> Development/Tools or Development/Languages maybe?
Done: Development/Tools

> 
> * %eclipse_base must be
>    %define eclipse_base %{_datadir}/eclipse
> Check: http://fedoraproject.org/wiki/Packaging/EclipsePlugins
> Otherwise the package will not build.
Not valid for Fedora 10. 

> 
> * No %doc files? Check the source thoroughly. Please list every applicable file
> in %doc.
There is really no doc provided.

> 
> * Use CPL for the license
Done.

> 
> * There shouldn't be both spaces and tabs in the same SPEC file. Please use one
> or the other. rpmlint is a good application which will warn you about such
> things and many other issues.
Fixed. Rpmlint do not show any warning.
Comment 3 Orcan Ogetbil 2008-11-25 21:11:51 EST
* Whenever you are uploading a new release, don't forget the bump the release number. This applies even during the review process.

* rpmlint says
     eclipse-shelled.noarch: W: no-documentation
     eclipse-shelled.src: W: strange-permission fetch-shelled.sh 0764

   ** At least these files need to go to %doc. 
         ./com.something.eclipse.shelled-feature/license.html
         ./com.something.eclipse.shelled-feature/cpl-v10.html
         ./com.something.eclipse.shelled.resources/about.html
     (Check the sample spec file at the eclipse guidelines)
     The other about.html files have the same content. 
     Other than this, what are all those manpages for?

   ** Afaik, we use 644 for source files.

* You do not need
     BuildRequires:    zip
     BuildRequires:    lzma

* We prefer %defattr(-,root,root,-)

* Use the %{version} macro whenever applicable (e.g. source0).
Comment 4 Alexander Kurtakov 2008-11-26 04:36:34 EST
Updated :

Spec URL:
http://akurtakov.fedorapeople.org/eclipse-shelled.spec
SRPM URL:
http://akurtakov.fedorapeople.org/eclipse-shelled-1.0.3-2.fc9.src.rpm

(In reply to comment #3)
> * Whenever you are uploading a new release, don't forget the bump the release
> number. This applies even during the review process.
> 
> * rpmlint says
>      eclipse-shelled.noarch: W: no-documentation
>      eclipse-shelled.src: W: strange-permission fetch-shelled.sh 0764
> 
>    ** At least these files need to go to %doc. 
>          ./com.something.eclipse.shelled-feature/license.html
>          ./com.something.eclipse.shelled-feature/cpl-v10.html
>          ./com.something.eclipse.shelled.resources/about.html
>      (Check the sample spec file at the eclipse guidelines)
>      The other about.html files have the same content. 
>      Other than this, what are all those manpages for?
> 
>    ** Afaik, we use 644 for source files.
Fixed.

> 
> * You do not need
>      BuildRequires:    zip
>      BuildRequires:    lzma
Fixed.

> 
> * We prefer %defattr(-,root,root,-)
Fixed.

> 
> * Use the %{version} macro whenever applicable (e.g. source0).
Fixed.
Comment 5 Alexander Kurtakov 2008-11-26 19:05:10 EST
Updated :

Spec URL:
http://akurtakov.fedorapeople.org/eclipse-shelled.spec
SRPM URL:
http://akurtakov.fedorapeople.org/eclipse-shelled-1.0.3-3.fc9.src.rpm

FIx %%doc handling.
Comment 6 Orcan Ogetbil 2008-11-27 09:41:26 EST
Thanks! Good to go.

---------------------------------------------------------
This package (eclipse-shelled) has been APPROVED by oget.
---------------------------------------------------------
Comment 7 Alexander Kurtakov 2008-11-27 10:03:41 EST
New Package CVS Request
=======================
Package Name: eclipse-shelled
Short Description: Eclipse Shell script editor
Owners: akurtakov
Branches: F-10
InitialCC: akurtakov
Comment 8 Kevin Fenzi 2008-12-01 15:53:38 EST
cvs done.