Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: eclipse-shelled - Shell script editor plugin for Eclipse
https://bugzilla.redhat.com/show_bug.cgi?id=470792
Summary: Review Request: eclipse-shelled - Shell script editor plugin for Eclipse Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: akurtako@redhat.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
Orcan 'oget' Ogetbil orcanbahri@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |orcanbahri@yahoo.com
--- Comment #1 from Orcan 'oget' Ogetbil orcanbahri@yahoo.com 2008-11-25 01:46:21 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
--- Comment #2 from Alexander Kurtakov akurtako@redhat.com 2008-11-25 04:40:34 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
--- Comment #3 from Orcan 'oget' Ogetbil orcanbahri@yahoo.com 2008-11-25 21:11:51 EDT --- * 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).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
Orcan 'oget' Ogetbil orcanbahri@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |orcanbahri@yahoo.com Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
--- Comment #4 from Alexander Kurtakov akurtako@redhat.com 2008-11-26 04:36:34 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
--- Comment #5 from Alexander Kurtakov akurtako@redhat.com 2008-11-26 19:05:10 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
Orcan 'oget' Ogetbil orcanbahri@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #6 from Orcan 'oget' Ogetbil orcanbahri@yahoo.com 2008-11-27 09:41:26 EDT --- Thanks! Good to go.
--------------------------------------------------------- This package (eclipse-shelled) has been APPROVED by oget. ---------------------------------------------------------
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
Alexander Kurtakov akurtako@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #7 from Alexander Kurtakov akurtako@redhat.com 2008-11-27 10:03:41 EDT --- New Package CVS Request ======================= Package Name: eclipse-shelled Short Description: Eclipse Shell script editor Owners: akurtakov Branches: F-10 InitialCC: akurtakov
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #8 from Kevin Fenzi kevin@tummy.com 2008-12-01 15:53:38 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=470792
Alexander Kurtakov akurtako@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
package-review@lists.fedoraproject.org