Bug 575480
| Summary: | Review Request: imvirt - Detects several virtualizations | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Miroslav Suchý <msuchy> |
| Component: | Package Review | Assignee: | Steve Traylen <steve.traylen> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | low | ||
| Version: | rawhide | CC: | fedora-package-review, kalevlember, notting, pahan, steve.traylen |
| Target Milestone: | --- | Flags: | steve.traylen:
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: | 2010-03-24 12:54:41 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Miroslav Suchý
2010-03-20 22:52:34 UTC
Source is not URL and no comments why and how get it. Fixed. Spec and srpm is on same location. Hi Miroslav,
First I think its much easier for reviewers if you always bump the release
numbers during the review.
Here's a review:
Review: imvirt
Date: 21st March 2011
Mock Build: F14, x86_64 builds.
* YES: rpmlint output
rpmlint SPECS/imvirt.spec RPMS/x86_64/imvirt-* SRPMS/imvirt-0.9.0-pre1.fc14.src.rpm
imvirt.x86_64: W: spelling-error Summary(en_US) virtualizations -> conceptualizations, visualizations, actualization
imvirt.x86_64: W: spelling-error %description -l en_US virtualization -> actualization, visualization, contextualization
imvirt.x86_64: W: spelling-error %description -l en_US virtualized -> ritualized, actualized, virtual
imvirt.src: W: spelling-error Summary(en_US) virtualizations -> conceptualizations, visualizations, actualization
imvirt.src: W: spelling-error %description -l en_US virtualization -> actualization, visualization, contextualization
imvirt.src: W: spelling-error %description -l en_US virtualized -> ritualized, actualized, virtual
3 packages and 1 specfiles checked; 0 errors, 6 warnings.
All of these words are in very common usage within this context.
They will become words soon enough.
* YES: Named according to the Package Naming Guidelines.
* YES: spec file name same as base package %{name}.
* NO: Packaging Guidelines.
The Source tar ball is imvirt-0.9.0-pre1.tar.gz which is presumably
a pre-release to 0.9.0. This needs to be handled in the release.
See: http://fedoraproject.org/wiki/PackageNamingGuidelines#Pre-Release_packages
* YES: Approved license in .spec file.
GPLv2+
* COMMENT: License on Source code.
CLearly licensed this way.
* YES: Include LICENSE file or similar if it exist.
COPYFILE file present.
* YES: Written in American English.
* YES: Spec file legible.
* YES: Included source must match upstream source.
$ md5sum imvirt-0.9.0-pre1.tar.gz ../SOURCES/imvirt-0.9.0-pre1.tar.gz
698d022b778aaf0d07ba67fa357da464 imvirt-0.9.0-pre1.tar.gz
698d022b778aaf0d07ba67fa357da464 ../SOURCES/imvirt-0.9.0-pre1.tar.gz
Is the Source URL possible to define exactly the example here?
http://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
* YES: Build on one architecture.
* YES: Not building on an architecture must highlighted.
* YES: Build dependencies must be listed in BuildRequires.
* YES: Handle locales properly.
* YES: ldconfig must be called on shared libs.
* YES: No bundled copies of system libraries.
* YES: Package must state why relocatable if relocatable.
* YES: A package must own all directories that it creates
* YES: No duplicate files in %files listings.
* YES: Permissions on files must be set properly. %defattr
* YES: %clean section contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
* YES: Each package must consistently use macros.
* YES: The package must contain code, or permissable content.
* YES: Large documentation files must go in a -doc subpackage.
No large docs.
* YES: %doc must not affect the runtime of the application.
* YES: Header files must be in a -devel package.
NO headers.
* YES: Static libraries must be in a -static package.
No statics.
* YES: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
No pkgconfig
* YES: Then library files that end in .so
(without suffix) must go in a -devel package.
No .sos
* YES: devel packages must require the exact base package
None
* YES: No .la libtool archives
None
* YES: GUI apps should have %{name}.desktop file
No gui
* YES: No files or directories already owned by other packages.
None are.
* YES: %install must run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
It does.
* YES: All filenames in rpm packages must be valid UTF-8.
Yes.
So just handling of the fact this is a pre-release
and if you can change the URL do so.
I think Vendor: IBH IT-Service GmbH (http://www.ibh.de/) should come out. SPEC: http://miroslav.suchy.cz/fedora/imvirt/ SRPM: http://miroslav.suchy.cz/fedora/imvirt/imvirt-0.9.0-0.1.pre1.el6.src.rpm I fixed that release tag according to guidelines. I removed Vendor tag. But I did not change Source0 url since that template does not work in this case. I tried http://downloads.sourceforge.net/imvirt/imvirt-0.9.0-pre1.tar.gz http://downloads.sourceforge.net/imvirt/imvirt-0.9.0.tar.gz and both returns 404 not found. So I will stick with that one which works.
APPROVED but could you add a "-p" to
install -m644 -D ImVirt.pm $RPM_BUILD_ROOT%{perl_vendorlib}/ImVirt.pm
post review to preserve the timestamp.
Steve
New Package CVS Request ======================= Package Name: imvirt Short Description: Detects several virtualizations Owners: msuchy Branches: F-12, F-13, EL-5 InitialCC: (In reply to comment #5) > But I did not change Source0 url since that template does not work in this > case. > I tried > http://downloads.sourceforge.net/imvirt/imvirt-0.9.0-pre1.tar.gz > http://downloads.sourceforge.net/imvirt/imvirt-0.9.0.tar.gz > and both returns 404 not found. So I will stick with that one which works. Try this one: http://downloads.sourceforge.net/apt-dater/%{name}-%{version}-pre1.tar.gz that url works. thx kalev. CVS done (by process-cvs-requests.py). |