Bug 818805

Updating metadata

Oops, wrong links. New ones:
spec: http://dl.dropbox.com/u/17870887/openerp-client/openerp-client.spec
srpm: http://dl.dropbox.com/u/17870887/openerp-client/openerp-client-6.1-1.fc16.src.rpm

I will take this review.

(Alex can you please use you fedorapeople.org account for hosting your specs/SRPMS?)

Sure. Unless it's a problem for you, I'll wait until there is a new set to publish. Let me know if also this is a problem, and I'll fix. Havn't really got used to use fedorapeople.org yet ;)

Done. Links at http://leamas.fedorapeople.org/openerp-client.

Hey Alec (apologies - misspelled your name before)

Just a few things to clarify before I dig in to this one.

 - you should prefix your sources with the package name (and version if relevant)
 - when providing an alternate license file you need to explicitly state where you got it from
 - I'm confused by the changelog. Where was this spec from originally? You need to cull the invalid change log entries. A single changelog entry will suffice if the spec was sourced out of Fedora, but even this is not necessary.

 - Rpmlint throws up an error against noarch:

rpmlint /home/bsjones/rpmbuild/SRPMS/openerp-client-6.1-1.fc17.src.rpm /home/bsjones/rpmbuild/RPMS/noarch/openerp-client-6.1-1.fc17.noarch.rpm
openerp-client.src: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
2 packages and 0 specfiles checked; 1 errors, 2 warnings.

 - use the %{name} macro wherever you can 
 - I like to see all of the docs/man/bin files defined explicitly in the %files section (I find it helps in the review). Its safer, and you can get a better picture of the installed RPM. The suffix for man files should be .* however

Sounds like a lot, but not really. All in pretty good shape

(In reply to comment #6)
> suffice if the spec was sourced out of Fedora, but even this is not

Sorry *outside* of Fedora

(In reply to comment #6)
Hi! Here we go:

Things partly fixed in 
spec: http://leamas.fedorapeople.org/openerp-client/openerp-client.spec
srpm: http://leamas.fedorapeople.org/openerp-client/openerp-client-6.1-2.fc16.src.rpm

>  - you should prefix your sources with the package name (and version if
> relevant)
Done for the licensing file.

>  - when providing an alternate license file you need to explicitly state
> where you got it from
There is no alternate license, just a license break-down with a possibly bad name. Renamed, see above

>  - I'm confused by the changelog. Where was this spec from originally? You
> need to cull the invalid change log entries. A single changelog entry will
> suffice if the spec was sourced out of Fedora, but even this is not
> necessary.
So am I. But this request is based on old bug 693425 (which I should have noted in the original request!) The changelog is from there. With this said, noone would be happier than me if we could drop it. Do you think it's OK, given this?

>  - Rpmlint throws up an error against noarch: 
[cut]
> openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
[cut]
This one beats me. I think its a rpmlint error, it somehow thinks anything named like mathplotlib is a "normal" lib. However, I really need this Requires:, there is no automatic resolver for python handling this, and I need the python module. 

>  - use the %{name} macro wherever you can
Done (although note that the guidelines explicitly says this is a matter of style).

>  - I like to see all of the docs/man/bin files defined explicitly in the
> %files section (I find it helps in the review). Its safer, and you can get a
> better picture of the installed RPM. The suffix for man files should be .*
> however
Done

(In reply to comment #8)

> >  - I'm confused by the changelog. Where was this spec from originally? You
> > need to cull the invalid change log entries. A single changelog entry will
> > suffice if the spec was sourced out of Fedora, but even this is not
> > necessary.
> So am I. But this request is based on old bug 693425 (which I should have
> noted in the original request!) The changelog is from there. With this said,
> noone would be happier than me if we could drop it. Do you think it's OK,
> given this?
> 

Wow! war and peace - yes, drop all of the change log - its not relevant.

> >  - Rpmlint throws up an error against noarch: 
> [cut]
> > openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
> [cut]
> This one beats me. I think its a rpmlint error, it somehow thinks anything
> named like mathplotlib is a "normal" lib. However, I really need this
> Requires:, there is no automatic resolver for python handling this, and I
> need the python module. 

You are right. I did a quick BZ search - this has come up a few times.

> 
> >  - use the %{name} macro wherever you can
> Done (although note that the guidelines explicitly says this is a matter of
> style).

True!

> 
> >  - I like to see all of the docs/man/bin files defined explicitly in the
> > %files section (I find it helps in the review). Its safer, and you can get a
> > better picture of the installed RPM. The suffix for man files should be .*
> > however
> Done

Thanks Alec. I'll try and finish this in the next day or so.

Dropped the changelog. Same links.

Do I dare to send a ping?

Apologies Alec, I have been caught up playing host to some friends from home. Should get to this today.

Just some comments in the Requires: that need dropping. This package is APPROVED


Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
[x]: MUST The spec file handles locales properly.
[x]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Requires correct, justified where necessary.
[!]: MUST Rpmlint output is silent.

rpmlint openerp-client-6.1-2.fc18.src.rpm

openerp-client.src: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint openerp-client-6.1-2.fc18.noarch.rpm

openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 1 errors, 1 warnings.


[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/bsjones/rpmbuild/SRPMS/openerp-client-6.1-1.tar.gz :
  MD5SUM this package     : f0d349bfa0f1d8979b36a64bd5204b0c
  MD5SUM upstream package : f0d349bfa0f1d8979b36a64bd5204b0c

[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[-]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: SHOULD Package functions as described.
[x]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD Scriptlets must be sane, if used.
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[-]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop
[!]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
[!]: MUST Rpmlint output is silent.

rpmlint openerp-client-6.1-2.fc18.src.rpm

openerp-client.src: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint openerp-client-6.1-2.fc18.noarch.rpm

openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 1 errors, 1 warnings.


See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint


Generated by fedora-review 0.1.3
External plugins:

Thanks for review!


New Package SCM Request
=======================
Package Name: openerp-client
Short Description: Business Applications Server Client
Owners: leamas
Branches: f16 f17
InitialCC:

Brendan, please ser fedora-review to +.

Apologies!

You shouldn't apologize, I should. It was I who somehow reset your '+' to '?' when setting fedora-cvs flag.

I have yet to make a cvs request without making a mistake. Seems utterly hard.

New Package SCM Request
=======================
Package Name: openerp-client
Short Description: Business Applications Server Client
Owners: leamas
Branches: f16 f17
InitialCC:

Git done (by process-git-requests).

Build OK for rawhide, f17, f16. Closing

Duplicate.