Bug 882704

Summary:	Review Request: otopi - oVirt Task Oriented Pluggable Installer/Implementation
Product:	[Fedora] Fedora	Reporter:	Alon Bar-Lev <alonbl>
Component:	Package Review	Assignee:	Douglas Schilling Landgraf <dougsland>
Status:	CLOSED DEFERRED	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	a.badger, alonbl, bazulay, dougsland, i, iheim, marcelo.barbosa, mgoldboi, oschreib, rc040203, rjones, yzaslavs
Target Milestone:	---	Flags:	dougsland: fedora-review+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2014-01-27 14:17:44 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:
Bug Depends On:
Bug Blocks:	882711

Description Alon Bar-Lev 2012-12-02 19:23:09 UTC

Spec URL: https://github.com/downloads/alonbl/otopi/otopi.spec
SRPM URL: https://github.com/downloads/alonbl/otopi/otopi-0.0.0-0.0.master.fc17.src.rpm
Description: A dependency of oVirt project version 3.2 release, dependency of ovirt-engine.
Fedora Account System Username: alonbl

Comment 1 Alon Bar-Lev 2012-12-02 19:26:03 UTC

This is my first package and I need sponsor.

I am the upstream maintainer.

Build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4749118

Comment 2 Alon Bar-Lev 2012-12-02 19:41:06 UTC

Attach the report and my notes '-->', I hope I got this correctly.
This is a pre-release, however, I would like to know that all OK.
Thanks!

---

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

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
[!]: Packages have proper BuildRequires/Requires on jpackage-utils
See: https://fedoraproject.org/wiki/Packaging:Java
--> This is false positive, we only need jpackage-utils for the java subpackage.

[!]: Package DOES NOT use %update_maven_depmap in %post/%postun
See: https://fedoraproject.org/wiki/Packaging:Java#add_maven_depmap_macro

[!]: Fully versioned dependency in subpackages, if present.
     Note: Missing: 'Requires: %%{name} =' in: %package devel
See: http://fedoraproject.org/wiki/Packaging/Guidelines#RequiringBasePackage
--> This is false positive as the devel package depends on the java package which depends on the base package correctly.


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
--> LGPLv2.1+
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package contains no bundled libraries.
[?]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in %package
     javadoc, %package devel
[x]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[x]: 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.
--> This is false positive as the devel package depends on the java package which depends on the base package correctly.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "LGPL (v2.1 or later)". 1 files have unknown license. Detailed output of
     licensecheck in /home/test1/review-otopi/licensecheck.txt
--> False positive.
[x]: License file installed when any subpackage combination is installed.
--> Except for the javadoc subpackage.
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named using only allowed ASCII characters.
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: Package do not use a name that already exist
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package installs properly.
[x]: Package is not relocatable.
[x]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file is legible and written in American English.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[-]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 51200 bytes in 6 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[!]: Packages have proper BuildRequires/Requires on jpackage-utils
--> This is false positive, we only need jpackage-utils for the java subpackage.
[!]: Fully versioned dependency in subpackages, if present.
     Note: Missing: 'Requires: %%{name} =' in: %package devel
--> This is false positive as the devel package depends on the java package which depends on the base package correctly.
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadoc subpackages have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
[x]: Bundled jar/class files should be removed before build

Maven:
[!]: Pom files have correct add_maven_depmap call
     Note: Some add_maven_depmap calls found. Please check if they are correct
--> This is false positive.
[x]: Old add_to_maven_depmap macro is not being used
[x]: Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-
     utils for %update_maven_depmap macro
[x]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[!]: Package DOES NOT use %update_maven_depmap in %post/%postun
--> This is false positive, we only need %update_maven_depmap for the java subpackage.
[x]: Packages use %{_mavenpomdir} instead of %{_datadir}/maven2/poms

Python:
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep
[x]: Python eggs must not download any dependencies during the build process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[?]: Buildroot is not present
     Note: Buildroot: present but not needed
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[-]: 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]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: Package functions as described.
[x]: Latest version is packaged.
[-]: Package does not include license text files separate from upstream.
[x]: The placement of pkgconfig(.pc) files are correct.
[-]: Scriptlets must be sane, if used.
[?]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[x]: SourceX / PatchY prefixed with %{name}.
[x]: SourceX is a working URL.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

Java:
[x]: Package has BuildArch: noarch (if possible)
[x]: Package uses upstream build method (ant/maven/etc.)

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: otopi-devel-0.0.0-0.0.master.fc17.noarch.rpm
          otopi-javadoc-0.0.0-0.0.master.fc17.noarch.rpm
          otopi-java-0.0.0-0.0.master.fc17.noarch.rpm
          otopi-0.0.0-0.0.master.fc17.noarch.rpm
          otopi-0.0.0-0.0.master.fc17.src.rpm
otopi-devel.noarch: W: summary-not-capitalized C otopi development components
otopi-devel.noarch: W: no-documentation
otopi-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
otopi-java.noarch: W: summary-not-capitalized C otopi java support
otopi-java.noarch: W: no-documentation
otopi.noarch: W: spelling-error Summary(en_US) oVirt -> overt
otopi.noarch: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
otopi.noarch: W: incoherent-version-in-changelog 0.0.0-0 ['0.0.0-0.0.master.fc17', '0.0.0-0.0.master']
otopi.noarch: E: non-executable-script /usr/share/otopi/plugins/otopi/packagers/miniyum.py 0644L /usr/bin/python
otopi.noarch: W: no-manual-page-for-binary otopi
otopi.src: W: spelling-error Summary(en_US) oVirt -> overt
otopi.src: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
otopi.src:131: W: macro-in-comment %{_datadir}
otopi.src:131: W: macro-in-comment %{name}
otopi.src: W: invalid-url Source0: http://resources.ovirt.org/releases/stable/src/otopi-0.0.0_master.tar.gz HTTP Error 404: Not Found
5 packages and 0 specfiles checked; 1 errors, 14 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint otopi-java otopi otopi-javadoc otopi-devel
otopi-java.noarch: W: summary-not-capitalized C otopi java support
--> otopi - as-is
otopi-java.noarch: W: no-documentation
--> unclear?
otopi.noarch: W: spelling-error Summary(en_US) oVirt -> overt
--> False positive.
otopi.noarch: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
--> oVirt - as-is
otopi.noarch: W: incoherent-version-in-changelog 0.0.0-0 ['0.0.0-0.0.master.fc17', '0.0.0-0.0.master']
--> will modify.
otopi.noarch: E: non-executable-script /usr/share/otopi/plugins/otopi/packagers/miniyum.py 0644L /usr/bin/python
--> not required
otopi.noarch: W: no-manual-page-for-binary otopi
--> not required
otopi-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
--> false positive
otopi-devel.noarch: W: summary-not-capitalized C otopi development components
--> otopi - as-is
otopi-devel.noarch: W: no-documentation
--> unclear?
4 packages and 0 specfiles checked; 1 errors, 9 warnings.
# echo 'rpmlint-done:'



Requires
--------
otopi-devel-0.0.0-0.0.master.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    /bin/sh  
    otopi-java = 0.0.0-0.0.master.fc17
    python(abi) = 2.7

otopi-javadoc-0.0.0-0.0.master.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    jpackage-utils  

otopi-java-0.0.0-0.0.master.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    /bin/sh  
    apache-commons-logging  
    java  
    jpackage-utils  
    otopi = 0.0.0-0.0.master.fc17

otopi-0.0.0-0.0.master.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    /bin/sh  
    python  
    python(abi) = 2.7



Provides
--------
otopi-devel-0.0.0-0.0.master.fc17.noarch.rpm:
    
    otopi-devel = 0.0.0-0.0.master.fc17

otopi-javadoc-0.0.0-0.0.master.fc17.noarch.rpm:
    
    otopi-javadoc = 0.0.0-0.0.master.fc17

otopi-java-0.0.0-0.0.master.fc17.noarch.rpm:
    
    mvn(org.ovirt.otopi:otopi)  
    otopi-java = 0.0.0-0.0.master.fc17

otopi-0.0.0-0.0.master.fc17.noarch.rpm:
    
    otopi = 0.0.0-0.0.master.fc17



MD5-sum check
-------------


Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-17-x86_64
Command line :/bin/fedora-review --rpm-spec --name=/home/test1/rpmbuild/SRPMS/otopi-0.0.0-0.0.master.fc17.src.rpm

Comment 3 Ofer Schreiber 2012-12-19 13:34:19 UTC

Two small issues:
1. The upstream source is missing
2. According to the packaging guidelines : "If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it."
since you're the upstream maintainer, I guess including the LICENSE should be easy

Comment 4 Alon Bar-Lev 2012-12-19 14:01:21 UTC

(In reply to comment #3)
> Two small issues:
> 1. The upstream source is missing

I waited for the review to be complete, before tagging.

> 2. According to the packaging guidelines : "If the source package does not
> include license text(s) as a separate file from upstream, the packager
> SHOULD query upstream to include it."
> since you're the upstream maintainer, I guess including the LICENSE should
> be easy

COPYING is available.

Thank you for review!

Comment 5 Alon Bar-Lev 2013-01-15 08:33:45 UTC

beta is available[1].
we can progress.
what is the next stage?

[1] http://resources.ovirt.org/releases/3.2/src/otopi-1.0.0_beta.tar.gz

Comment 6 Alon Bar-Lev 2013-01-15 08:35:17 UTC

koji build is available[1] with embedded spec file.

[1] http://koji.fedoraproject.org/koji/taskinfo?taskID=4869012

Comment 7 Douglas Schilling Landgraf 2013-06-11 23:22:28 UTC

Hi Alon, 

As you noticed, I have just jumped into this bugzilla and looks like it's out of date.

1) Cannot find the spec.
$ wget https://github.com/downloads/alonbl/otopi/otopi.spec
HTTP request sent, awaiting response... 404 Not Found
2013-06-11 20:02:18 ERROR 404: Not Found

2) I am pretty sure otopi 1.0.0 is not beta anymore :) 
  otopi-1.0.0_beta.tar.gz 

BTW, to get sponsored you need to do an informal package review [1]. 
Basically, you can pick up a random bugzilla that is requesting a package review and push your comments there (don't need to assign it) and share the link here.

To help you in this task, I can suggest:
- FedoraReview - Fedora Hosted
https://fedorahosted.org/FedoraReview/

- Package Review Guidelines
http://fedoraproject.org/wiki/Packaging:ReviewGuidelines

- Package Review Guidelines for Java
http://fedoraproject.org/wiki/Packaging:Java

- Package Review Guidelines for Python
http://fedoraproject.org/wiki/Packaging:Python


[1] https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Please let me know if you have any question.

Thanks
Douglas

Comment 8 Alon Bar-Lev 2013-06-12 08:23:05 UTC

Hello Douglas,

spec file is at upstream[1].

I will be happy if you can send patches to whatever you think that should be improved.

Thanks,
Alon

[1] http://gerrit.ovirt.org/gitweb?p=otopi.git;a=blob;f=otopi.spec.in;hb=HEAD

Comment 9 Christopher Meng 2013-09-12 14:18:36 UTC

ping Alon for new spec and srpm.

Comment 10 Alon Bar-Lev 2013-09-12 19:49:20 UTC

(In reply to Christopher Meng from comment #9)
> ping Alon for new spec and srpm.

Hi,

spec is always available upstream[1], tarball is always avaible[2], also you can find srpm at upstream repo[3].

However, as ovirt-engine is not going to be included in fedora any time soon, it is not essential that otopi/ovirt-host-deploy will.

But I will still appreciate a review!

Thanks!

[1] http://gerrit.ovirt.org/gitweb?p=otopi.git;a=blob;f=otopi.spec.in;hb=HEAD
[2] http://resources.ovirt.org/releases/nightly/src/
[3] http://resources.ovirt.org/releases/nightly/rpm/Fedora/19/SRPMS/

Comment 11 Ralf Corsepius 2013-09-13 03:15:18 UTC

(In reply to Alon Bar-Lev from comment #10)
> (In reply to Christopher Meng from comment #9)
> > ping Alon for new spec and srpm.

> spec is always available upstream[1], tarball is always avaible[2], also you
> can find srpm at upstream repo[3].
This is not how Fedora works. I conclude, you are not interested in submitting this package to Fedora, so this review request can be closed?

Comment 12 Alon Bar-Lev 2013-09-15 15:40:50 UTC

(In reply to Ralf Corsepius from comment #11)
> (In reply to Alon Bar-Lev from comment #10)
> > (In reply to Christopher Meng from comment #9)
> > > ping Alon for new spec and srpm.
> 
> > spec is always available upstream[1], tarball is always avaible[2], also you
> > can find srpm at upstream repo[3].
> This is not how Fedora works. I conclude, you are not interested in
> submitting this package to Fedora, so this review request can be closed?

I pointed to these artifacts exactly as requested, not sure how it is "not whow fedora works".

Let's try again.

spec[1], srpm[2].

fedora-review:
---
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Fully versioned dependency in subpackages, if present.
  Note: Missing: Requires: %{name} = %{version}-%{release} in otopi-java,
  otopi-devel
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#RequiringBasePackage

### FALSE POSITIVE? ###

===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[-]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in otopi-java
     , otopi-javadoc , otopi-devel
[x]: Package complies to the Packaging Guidelines
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "LGPL (v2.1 or later)". Detailed output of licensecheck in
     /home/user1/otopi/licensecheck.txt
[?]: License file installed when any subpackage combination is installed.
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[-]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 51200 bytes in 6 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: 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]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).

Java:
[x]: Packages have proper BuildRequires/Requires on jpackage-utils
     Note: Maven packages do not need to (Build)Require jpackage-utils. It is
     pulled in by maven-local
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadoc subpackages should not have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
[x]: Bundled jar/class files should be removed before build

Maven:
[x]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[x]: Pom files have correct Maven mapping
[x]: Maven packages should use new style packaging
[x]: Old add_to_maven_depmap macro is not being used
[x]: Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-
     utils for %update_maven_depmap macro
[x]: Package DOES NOT use %update_maven_depmap in %post/%postun
[x]: Packages use %{_mavenpomdir} instead of %{_datadir}/maven2/poms

Python:
[x]: Python eggs must not download any dependencies during the build process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[!]: Sources can be downloaded from URI in Source: tag
     Note: Could not download Source0
[ ]: Buildroot is not present
     Note: Buildroot: present but not needed
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[ ]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

Java:
[x]: Package uses upstream build method (ant/maven/etc.)
[x]: Packages are noarch unless they use JNI

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Rpmlint
-------
Checking: otopi-1.2.0-0.0.master.fc19.noarch.rpm
          otopi-java-1.2.0-0.0.master.fc19.noarch.rpm
          otopi-javadoc-1.2.0-0.0.master.fc19.noarch.rpm
          otopi-devel-1.2.0-0.0.master.fc19.noarch.rpm
otopi.noarch: W: spelling-error Summary(en_US) oVirt -> overt
otopi.noarch: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
otopi.noarch: W: incoherent-version-in-changelog 1.1.0-1 ['1.2.0-0.0.master.fc19', '1.2.0-0.0.master']
otopi.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python
otopi.noarch: W: no-manual-page-for-binary otopi
otopi-java.noarch: W: summary-not-capitalized C otopi java support
otopi-java.noarch: W: no-documentation
otopi-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
otopi-devel.noarch: W: summary-not-capitalized C otopi development components
otopi-devel.noarch: W: no-documentation
4 packages and 0 specfiles checked; 1 errors, 9 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint otopi otopi-java otopi-javadoc otopi-devel
otopi.noarch: W: spelling-error Summary(en_US) oVirt -> overt
otopi.noarch: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
otopi.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python
otopi.noarch: W: no-manual-page-for-binary otopi
otopi-java.noarch: W: summary-not-capitalized C otopi java support
otopi-java.noarch: W: no-documentation
otopi-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
otopi-devel.noarch: W: summary-not-capitalized C otopi development components
otopi-devel.noarch: W: no-documentation
4 packages and 0 specfiles checked; 1 errors, 8 warnings.
# echo 'rpmlint-done:'



Requires
--------
otopi (rpmlib, GLIBC filtered):
    /bin/sh
    python
    python(abi)

otopi-java (rpmlib, GLIBC filtered):
    apache-commons-logging
    java
    jpackage-utils
    otopi

otopi-javadoc (rpmlib, GLIBC filtered):
    jpackage-utils

otopi-devel (rpmlib, GLIBC filtered):
    /bin/sh
    otopi-java
    python(abi)



Provides
--------
otopi:
    otopi

otopi-java:
    mvn(org.ovirt.otopi:otopi)
    otopi-java

otopi-javadoc:
    otopi-javadoc

otopi-devel:
    otopi-devel



Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-19-x86_64
Command line :/bin/fedora-review -rn /home/user1/rpmbuild/SRPMS/otopi-1.2.0-0.0.master.fc19.src.rpm
---

[1] https://github.com/alonbl/fedora-files/blob/master/otopi.spec
[2] https://github.com/alonbl/fedora-files/blob/master/otopi-1.2.0-0.0.master.fc19.src.rpm

Comment 13 Ralf Corsepius 2013-09-16 05:08:22 UTC

(In reply to Alon Bar-Lev from comment #12)
> (In reply to Ralf Corsepius from comment #11)
> > (In reply to Alon Bar-Lev from comment #10)
> > > (In reply to Christopher Meng from comment #9)
> > > > ping Alon for new spec and srpm.
> > 
> > > spec is always available upstream[1], tarball is always avaible[2], also you
> > > can find srpm at upstream repo[3].
> > This is not how Fedora works. I conclude, you are not interested in
> > submitting this package to Fedora, so this review request can be closed?
> 
> I pointed to these artifacts exactly as requested, not sure how it is "not
> whow fedora works".
The package submitter is supposed to update BZs each time, he changes something in his submissions. Just pointing reviewers" to a rolling git to have them pick up the "spec/tarball of the day" is not going to fly anywhere.

Comment 14 Alon Bar-Lev 2013-09-16 06:17:44 UTC

(In reply to Ralf Corsepius from comment #13)
> > I pointed to these artifacts exactly as requested, not sure how it is "not
> > whow fedora works".
> The package submitter is supposed to update BZs each time, he changes
> something in his submissions. Just pointing reviewers" to a rolling git to
> have them pick up the "spec/tarball of the day" is not going to fly anywhere.

Well, considering the time I got no response, this would have a void effort...

Just for me not wasting your time, this package should be prepared to meet the standard of Fefora, so that when ovirt-engine package finish review it will be ready to inclusion too.

Comment 15 Douglas Schilling Landgraf 2013-09-17 03:26:52 UTC

Hi,

Thanks for update/share the spec and srpm, I will start the new review soon.

Comment 16 Douglas Schilling Landgraf 2013-09-17 18:52:24 UTC

Hi,

I have few comments before the approval:

* I cannot see the source for otopi-1.2.0 at http://resources.ovirt.org/releases/3.3/src/
  You might want to check the below link as well: http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control

* Please consider using macros like version for Source and prep.
  http://resources.ovirt.org/releases/3.3/src/%{name}-1.2.0_master.tar.gz

  %prep
  %setup -q -n %{name}-1.2.0_master

* If otopi 1.2.0 is already released use release Release: 1.master (or similar) if not
  Release 0.1.master (or similar)
  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag

* %changelog
  Keep changelog updated with spec changes.

* Javadoc subpackages should not have Requires: jpackage-utils
  Note: jpackage-utils requires are automatically generated by the buildsystem
  See: https://fedoraproject.org/wiki/Packaging:Java

* Please check rpmlint output:
  otopi.noarch: W: incoherent-version-in-changelog 1.1.0-1 ['1.2.0-0.0.master.fc19', '1.2.0-0.0.master']
  otopi.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python
  otopi.src: W: strange-permission otopi.spec 0640L
  otopi.src: W: strange-permission otopi-1.2.0_master.tar.gz 0640L

* No known owner of /usr/share/java/otopi

* %{python_sitelib}/%{name}/codegen/ if this module intend to be loaded by other code, you can leave it in main. Otherwise, use %exclude to remove it from main. Also, it doesn't have C headers or pkgconfig files, you might want to create a -devtools or -plugin-devel-tools subpackage instead of -devel. If codegen is only used when building the otopi package itself, you don't even have to include it in a package.

Comment 17 Alon Bar-Lev 2013-09-17 19:06:05 UTC

(In reply to Douglas Schilling Landgraf from comment #16)
> Hi,
> 
> I have few comments before the approval:
> 
> * I cannot see the source for otopi-1.2.0 at
> http://resources.ovirt.org/releases/3.3/src/
>   You might want to check the below link as well:
> http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
> 
> * Please consider using macros like version for Source and prep.
>   http://resources.ovirt.org/releases/3.3/src/%{name}-1.2.0_master.tar.gz
> 
>   %prep
>   %setup -q -n %{name}-1.2.0_master

fedora repo will contain only released tarballs, this will be %{name}-%{version}.

> * If otopi 1.2.0 is already released use release Release: 1.master (or
> similar) if not
>   Release 0.1.master (or similar)
>   https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag

otopi-1.2.0 is not released.
 
> * %changelog
>   Keep changelog updated with spec changes.

when imported into fedora repo it will be updated every commit.

> * Javadoc subpackages should not have Requires: jpackage-utils
>   Note: jpackage-utils requires are automatically generated by the
> buildsystem
>   See: https://fedoraproject.org/wiki/Packaging:Java

not sure where this comes from, my spec I sent in comment#13 does not contain that.
 
> * Please check rpmlint output:
>   otopi.noarch: W: incoherent-version-in-changelog 1.1.0-1
> ['1.2.0-0.0.master.fc19', '1.2.0-0.0.master']
>   otopi.noarch: E: non-executable-script
> /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python
>   otopi.src: W: strange-permission otopi.spec 0640L
>   otopi.src: W: strange-permission otopi-1.2.0_master.tar.gz 0640L

Did.

Old change log will be removed.
miniyum.py is executed so that it can be used as standalone script for problem determination.
the strange permissions at source tarball will be fixed when packaged by koji.

> * No known owner of /usr/share/java/otopi

Fixed.

> * %{python_sitelib}/%{name}/codegen/ if this module intend to be loaded by
> other code, you can leave it in main. Otherwise, use %exclude to remove it
> from main. Also, it doesn't have C headers or pkgconfig files, you might
> want to create a -devtools or -plugin-devel-tools subpackage instead of
> -devel. If codegen is only used when building the otopi package itself, you
> don't even have to include it in a package.

It is used by other tools, so probably you mean rename it to -devtools, quite complex as we now need to obsolete the -devel.

Comment 18 Toshio Ernie Kuratomi 2013-09-17 20:03:50 UTC

(In reply to Alon Bar-Lev from comment #17)
> (In reply to Douglas Schilling Landgraf from comment #16)
> > Hi,
> > 
> > I have few comments before the approval:
> > 
> > * I cannot see the source for otopi-1.2.0 at
> > http://resources.ovirt.org/releases/3.3/src/
> >   You might want to check the below link as well:
> > http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
> > 
> > * Please consider using macros like version for Source and prep.
> >   http://resources.ovirt.org/releases/3.3/src/%{name}-1.2.0_master.tar.gz
> > 
> >   %prep
> >   %setup -q -n %{name}-1.2.0_master
> 
> fedora repo will contain only released tarballs, this will be
> %{name}-%{version}.
> 
Using macros for the tarball are optional but I'm not sure that you can make this guarantee about released tarballs.  If all goes well, the package may outlive you as the maintainer.  You'd have no control over what another maintainer would do.

beyond that, if the Fedora packages are only going to be based on released tarballs then you should be creating this package for review based on releases, not on a snapshot.

> > * If otopi 1.2.0 is already released use release Release: 1.master (or
> > similar) if not
> >   Release 0.1.master (or similar)
> >   https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag
> 
> otopi-1.2.0 is not released.
>
Yeah, so your Release tag should probably start with 0.1  rather than 0.0.
  
> > * %changelog
> >   Keep changelog updated with spec changes.
> 
> when imported into fedora repo it will be updated every commit.
> 
This needs to begin now.  %changelog during package review helps everyone communicate.

>  
> > * Please check rpmlint output:
> >   otopi.noarch: W: incoherent-version-in-changelog 1.1.0-1
> > ['1.2.0-0.0.master.fc19', '1.2.0-0.0.master']
> >   otopi.noarch: E: non-executable-script
> > /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python
> >   otopi.src: W: strange-permission otopi.spec 0640L
> >   otopi.src: W: strange-permission otopi-1.2.0_master.tar.gz 0640L
> 
> Did.
> 
> Old change log will be removed.

You don't mean removed, correct?  Just a new one added to reflect the current version?  Some people prune the old entries from changelogs after many many years but a package under review never falls under that criteria.

> miniyum.py is executed so that it can be used as standalone script for
> problem determination.

rpmlint is complaining because the file is *not* executable, though.  It is mode 0644 but it contains a shebang line.  Sometimes these complaints can be ignored but sometimes they point out that a file should be made executable (0755) or have the shebang removed.

> 
> > * %{python_sitelib}/%{name}/codegen/ if this module intend to be loaded by
> > other code, you can leave it in main. Otherwise, use %exclude to remove it
> > from main. Also, it doesn't have C headers or pkgconfig files, you might
> > want to create a -devtools or -plugin-devel-tools subpackage instead of
> > -devel. If codegen is only used when building the otopi package itself, you
> > don't even have to include it in a package.
> 
> It is used by other tools, so probably you mean rename it to -devtools,
> quite complex as we now need to obsolete the -devel.

This package is not in Fedora yet so you don't officially need to do that.  If you do want to do that it isn't complex.  It's an Obsoletes and Provides with a proper version.

Comment 19 Alon Bar-Lev 2013-09-17 20:37:15 UTC

(In reply to Toshio Ernie Kuratomi from comment #18)
> (In reply to Alon Bar-Lev from comment #17)
> > (In reply to Douglas Schilling Landgraf from comment #16)
> > > Hi,
> > > 
> > > I have few comments before the approval:
> > > 
> > > * I cannot see the source for otopi-1.2.0 at
> > > http://resources.ovirt.org/releases/3.3/src/
> > >   You might want to check the below link as well:
> > > http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
> > > 
> > > * Please consider using macros like version for Source and prep.
> > >   http://resources.ovirt.org/releases/3.3/src/%{name}-1.2.0_master.tar.gz
> > > 
> > >   %prep
> > >   %setup -q -n %{name}-1.2.0_master
> > 
> > fedora repo will contain only released tarballs, this will be
> > %{name}-%{version}.
> > 
> Using macros for the tarball are optional but I'm not sure that you can make
> this guarantee about released tarballs.  If all goes well, the package may
> outlive you as the maintainer.  You'd have no control over what another
> maintainer would do.
> 
> beyond that, if the Fedora packages are only going to be based on released
> tarballs then you should be creating this package for review based on
> releases, not on a snapshot.

I thought we prepare for next release to be potentially included. But OK, I reverted to last release.
   
> > > * %changelog
> > >   Keep changelog updated with spec changes.
> > 
> > when imported into fedora repo it will be updated every commit.
> > 
> This needs to begin now.  %changelog during package review helps everyone
> communicate.

For this reason I use git...
But done :)
 
> >  
> > > * Please check rpmlint output:
> > >   otopi.noarch: W: incoherent-version-in-changelog 1.1.0-1
> > > ['1.2.0-0.0.master.fc19', '1.2.0-0.0.master']
> > >   otopi.noarch: E: non-executable-script
> > > /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python
> > >   otopi.src: W: strange-permission otopi.spec 0640L
> > >   otopi.src: W: strange-permission otopi-1.2.0_master.tar.gz 0640L
> > 
> > Did.
> > 
> > Old change log will be removed.
> 
> You don't mean removed, correct?  Just a new one added to reflect the
> current version?  Some people prune the old entries from changelogs after
> many many years but a package under review never falls under that criteria.
> 
> > miniyum.py is executed so that it can be used as standalone script for
> > problem determination.
> 
> rpmlint is complaining because the file is *not* executable, though.  It is
> mode 0644 but it contains a shebang line.  Sometimes these complaints can be
> ignored but sometimes they point out that a file should be made executable
> (0755) or have the shebang removed.

I prefer to ignore.
 
> > 
> > > * %{python_sitelib}/%{name}/codegen/ if this module intend to be loaded by
> > > other code, you can leave it in main. Otherwise, use %exclude to remove it
> > > from main. Also, it doesn't have C headers or pkgconfig files, you might
> > > want to create a -devtools or -plugin-devel-tools subpackage instead of
> > > -devel. If codegen is only used when building the otopi package itself, you
> > > don't even have to include it in a package.
> > 
> > It is used by other tools, so probably you mean rename it to -devtools,
> > quite complex as we now need to obsolete the -devel.
> 
> This package is not in Fedora yet so you don't officially need to do that. 
> If you do want to do that it isn't complex.  It's an Obsoletes and Provides
> with a proper version.

Done.

Thanks!

Comment 20 Alon Bar-Lev 2013-09-17 20:39:01 UTC

Updated artifacts.

- use release tarball
- rename devel->devtools
- own %{_javadir}/%{name}
- some more fixups to be more similar to upstream


spec[1]
srpm[2]
review report[3]

Thanks!

[1] https://github.com/alonbl/fedora-files/blob/master/otopi.spec
[2] https://github.com/alonbl/fedora-files/blob/master/otopi-1.1.1-1.src.rpm
[3] https://github.com/alonbl/fedora-files/blob/master/review.txt

Comment 21 Douglas Schilling Landgraf 2013-09-18 03:58:22 UTC

Hi Alon,

- codegen still available in the main package, please use %exclude macro.

- On %install session you only need rm -rf "%{buildroot}" if you are going to ship otopi for EPEL5.

Also, please consider:

- Use the macro %{dist} into Release

- Replace macros in %changelog
  otopi.src:132: W: macro-in-%changelog %{_javadir}
  otopi.src:132: W: macro-in-%changelog %{name}

- spaces versus tabs:
  otopi.src:74: W: mixed-use-of-spaces-and-tabs (spaces: line 74, tab: line 20)

Finally, I think you can use all the comments here in this bugzilla as benefit to update your other review request (BZ#882711) as well.

Thanks

Comment 22 Alon Bar-Lev 2013-09-18 07:31:10 UTC

(In reply to Douglas Schilling Landgraf from comment #21)
> Hi Alon,
> 
> - codegen still available in the main package, please use %exclude macro.

Sorry! I forgot the most important thing!
Fixed.

> - On %install session you only need rm -rf "%{buildroot}" if you are going
> to ship otopi for EPEL5.

I do for now.
 
> Also, please consider:
> 
> - Use the macro %{dist} into Release

Sure.

> - Replace macros in %changelog
>   otopi.src:132: W: macro-in-%changelog %{_javadir}
>   otopi.src:132: W: macro-in-%changelog %{name}

Done.

> - spaces versus tabs:
>   otopi.src:74: W: mixed-use-of-spaces-and-tabs (spaces: line 74, tab: line
> 20)

Why don't I get this error?
I do not understand where it comes from.
 
> Finally, I think you can use all the comments here in this bugzilla as
> benefit to update your other review request (BZ#882711) as well.

Will do, thanks!

Comment 23 Alon Bar-Lev 2013-09-18 07:33:30 UTC

Updated artifacts.

- re-add dist to Release.
- exclude codegen from main package.

spec[1]
srpm[2]
review report[3]

Thanks!

[1] https://github.com/alonbl/fedora-files/blob/master/otopi.spec
[2] https://github.com/alonbl/fedora-files/blob/master/otopi-1.1.1-2.fc19.src.rpm
[3] https://github.com/alonbl/fedora-files/blob/master/otopi-review.txt

Comment 24 Alon Bar-Lev 2013-09-18 07:34:12 UTC

I also don't understand the following from review:

Issues:
=======
- Fully versioned dependency in subpackages, if present.
  Note: Missing: Requires: %{name} = %{version}-%{release} in otopi-java,
  otopi-devtools
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#RequiringBasePackage

Comment 25 Douglas Schilling Landgraf 2013-09-18 12:57:50 UTC

(In reply to Alon Bar-Lev from comment #24)
> I also don't understand the following from review:
> 
> Issues:
> =======
> - Fully versioned dependency in subpackages, if present.
>   Note: Missing: Requires: %{name} = %{version}-%{release} in otopi-java,
>   otopi-devtools
>   See:
> http://fedoraproject.org/wiki/Packaging/Guidelines#RequiringBasePackage

> - spaces versus tabs:
>   otopi.src:74: W: mixed-use-of-spaces-and-tabs (spaces: line 74, tab: line
> 20)

> Why don't I get this error?
> I do not understand where it comes from.

For all above, might be your version of rpmlint or/and fedora-review version. I see that you have it correct in the spec.

From some of your comments:
Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-19-x86_64

Here:
rpmlint-1.5-1.fc19.noarch 
fedora-review version 0.5.0 920221d 2013-08-30 11:27:49 +0200

Comment 26 Douglas Schilling Landgraf 2013-09-18 16:44:12 UTC

Hi Alon,

Package got approved, you can check the review below.

At this moment, please follow the process from: 
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

If you have any questions please let me know.

*** Thanks all people that made comments in this bugzilla and helped to get this package soon shipped officially on Fedora repo.

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

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "LGPL (v2.1 or later)". Detailed output of licensecheck in
     /home/fedora/otopi/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 51200 bytes in 6 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
Java:
[x]: Packages have proper BuildRequires/Requires on jpackage-utils
     Note: Maven packages do not need to (Build)Require jpackage-utils. It is
     pulled in by maven-local
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadoc subpackages should not have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
[x]: Bundled jar/class files should be removed before build

Maven:
[x]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[x]: Pom files have correct Maven mapping
[x]: Maven packages should use new style packaging
[x]: Old add_to_maven_depmap macro is not being used
[x]: Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-
     utils for %update_maven_depmap macro
[x]: Package DOES NOT use %update_maven_depmap in %post/%postun
[x]: Packages use %{_mavenpomdir} instead of %{_datadir}/maven2/poms

Python:
[x]: Python eggs must not download any dependencies during the build process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[x]: Buildroot is not present
     Note: Buildroot: present but not needed - will ship EL5 as comment #22
[-]: 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]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in otopi-
     javadoc (Please check this note)
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

Java:
[x]: Package uses upstream build method (ant/maven/etc.)
[x]: Packages are noarch unless they use JNI

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: otopi-1.1.1-2.fc19.noarch.rpm
          otopi-java-1.1.1-2.fc19.noarch.rpm
          otopi-javadoc-1.1.1-2.fc19.noarch.rpm
          otopi-devtools-1.1.1-2.fc19.noarch.rpm
          otopi-1.1.1-2.fc19.src.rpm
otopi.noarch: W: spelling-error Summary(en_US) oVirt -> overt
otopi.noarch: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
otopi.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python - Ignored as comment #19
otopi.noarch: W: no-manual-page-for-binary otopi
otopi-java.noarch: W: summary-not-capitalized C otopi java support
otopi-java.noarch: W: no-documentation
otopi-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
otopi-devtools.noarch: W: summary-not-capitalized C otopi development tools
otopi-devtools.noarch: W: no-documentation
otopi.src: W: spelling-error Summary(en_US) oVirt -> overt
otopi.src: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
5 packages and 0 specfiles checked; 1 errors, 10 warnings.
Rpmlint (installed packages)
----------------------------
# rpmlint otopi otopi-java otopi-devtools otopi-javadoc
otopi.noarch: W: spelling-error Summary(en_US) oVirt -> overt
otopi.noarch: W: summary-not-capitalized C oVirt Task Oriented Pluggable Installer/Implementation (otopi)
otopi.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/otopi/miniyum.py 0644L /usr/bin/python - Ignored as comment #19
otopi.noarch: W: no-manual-page-for-binary otopi
otopi-java.noarch: W: summary-not-capitalized C otopi java support
otopi-java.noarch: W: no-documentation
otopi-devtools.noarch: W: summary-not-capitalized C otopi development tools
otopi-devtools.noarch: W: no-documentation
otopi-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
4 packages and 0 specfiles checked; 1 errors, 8 warnings.
# echo 'rpmlint-done:'



Requires
--------
otopi (rpmlib, GLIBC filtered):
    /bin/sh
    python
    python(abi)

otopi-java (rpmlib, GLIBC filtered):
    apache-commons-logging
    java
    jpackage-utils
    otopi
otopi-devtools (rpmlib, GLIBC filtered):
    /bin/sh
    otopi
    python(abi)

otopi-javadoc (rpmlib, GLIBC filtered):
    jpackage-utils



Provides
--------
otopi:
    otopi

otopi-java:
    mvn(org.ovirt.otopi:otopi)
    otopi-java

otopi-devtools:
    otopi-devel
    otopi-devtools

otopi-javadoc:
   otopi-javadoc



Source checksums
----------------
http://resources.ovirt.org/releases/3.3/src/otopi-1.1.1.tar.gz :
  CHECKSUM(SHA256) this package     : 1286dba1c5828204208924030e475449ac7fcd4ba0784d461af8af03be75369f
  CHECKSUM(SHA256) upstream package : 1286dba1c5828204208924030e475449ac7fcd4ba0784d461af8af03be75369f


Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30
Command line :/usr/bin/fedora-review --rpm-spec -n ./otopi-1.1.1-2.fc19.src.rpm
Buildroot used: fedora-19-x86_64
Active plugins: Python, Shell-api, Generic, Java
Disabled plugins: C/C++, SugarActivity, Perl, R, PHP, Ruby
Disabled flags: EPEL5, EXARCH, DISTTAG

Final status: APPROVED

Comment 27 Alon Bar-Lev 2013-09-18 18:46:53 UTC

Thanks all!

Now we wait until ovirt-engine finishes applicatoin.

Comment 28 Juan Hernández 2013-09-18 19:46:27 UTC

The ovirt-engine package was approved on Apr 2012, and it isn't a dependency of this package. What do you need to wait for?

Comment 29 Alon Bar-Lev 2013-09-18 20:09:06 UTC

(In reply to Juan Hernández from comment #28)
> The ovirt-engine package was approved on Apr 2012, and it isn't a dependency
> of this package. What do you need to wait for?

I wait for >=3.2, this is where otopi was introduced as dependency.

Comment 30 Douglas Schilling Landgraf 2013-09-19 00:44:36 UTC

(In reply to Alon Bar-Lev from comment #29)
> (In reply to Juan Hernández from comment #28)
> > The ovirt-engine package was approved on Apr 2012, and it isn't a dependency
> > of this package. What do you need to wait for?
> 
> I wait for >=3.2, this is where otopi was introduced as dependency.

Hi Alon, 

I do believe you should reconsider, your package is independent and approved by fedora packaging guidelines. I would suggest you to continue the process to get otopi ready on Fedora.

Comment 31 Alon Bar-Lev 2013-09-19 07:22:49 UTC

(In reply to Douglas Schilling Landgraf from comment #30)
> (In reply to Alon Bar-Lev from comment #29)
> > (In reply to Juan Hernández from comment #28)
> > > The ovirt-engine package was approved on Apr 2012, and it isn't a dependency
> > > of this package. What do you need to wait for?
> > 
> > I wait for >=3.2, this is where otopi was introduced as dependency.
> 
> Hi Alon, 
> 
> I do believe you should reconsider, your package is independent and approved
> by fedora packaging guidelines. I would suggest you to continue the process
> to get otopi ready on Fedora.

What are the next stages?

I do not see any benefit in publicizing this one when no other package in fedora requires it.

Comment 32 Douglas Schilling Landgraf 2013-09-19 13:05:42 UTC

(In reply to Alon Bar-Lev from comment #31)
> (In reply to Douglas Schilling Landgraf from comment #30)
> > (In reply to Alon Bar-Lev from comment #29)
> > > (In reply to Juan Hernández from comment #28)
> > > > The ovirt-engine package was approved on Apr 2012, and it isn't a dependency
> > > > of this package. What do you need to wait for?
> > > 
> > > I wait for >=3.2, this is where otopi was introduced as dependency.
> > 
> > Hi Alon, 
> > 
> > I do believe you should reconsider, your package is independent and approved
> > by fedora packaging guidelines. I would suggest you to continue the process
> > to get otopi ready on Fedora.
> 
> What are the next stages?

After your package is approved by the Package Review Process, you need to request for a repository to be created for your package. Please copy this template into the Bugzilla comment of your bug that's been passed for review, and set fedora-cvs flag to ?

Example:
---------

New Package SCM Request
=======================
Package Name: otopi
Short Description: summary of package
Owners: alonbl
Branches: f18 f19 f20 el6
InitialCC: people_fas_account_that_you_want_in_cc

After the above step you need to import your package to the branches created using fedpkg tool (will be required the ssh key from you FAS account).

More info: 
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner


Example:
----------

$ fedpkg clone otopi
$ git branch -a (to show the branches) - master is rawhide
$ fedpkg import <srpm>
$ fedpkg commit -p
$ fedpkg build

More info:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Import.2C_Commit.2Cand_Build_Your_Package

Updating others branches
--------------------------

$ fedpkg switch-branch BRANCH (e.g. f19)

(here you can use git merge or the same steps as above)

More info:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Update_Your_Branches_.28if_desired.29


Finally submit Package as Update in Bodhi
--------------------------------------------

Note: Do not submit "master" (rawhide) packages via bodhi.

$ fedpkg update

More info: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Submit_Package_as_Update_in_Bodhi

> 
> I do not see any benefit in publicizing this one when no other package in
> fedora requires it.

The main one is that when ovirt-engine on Fedora repo get into next version and requires otopi it will be already available.

Please let me know if you have any additional questions.

Cheers
Douglas

Comment 33 Alon Bar-Lev 2013-09-19 13:45:36 UTC

(In reply to Douglas Schilling Landgraf from comment #32)
> > 
> > I do not see any benefit in publicizing this one when no other package in
> > fedora requires it.
> 
> The main one is that when ovirt-engine on Fedora repo get into next version
> and requires otopi it will be already available.

Thanks, I prefer to wait and see if, when and which version of ovirt-engine is added before investing more resources of you/fedora and mine.

What important is that we are ready.

Comment 34 Richard W.M. Jones 2013-09-19 13:48:21 UTC

I'm slightly incredulous about what I'm reading here.

The reason you should put it in Fedora is because Fedora
is the upstream of RHEL.  ALL development should be done
first in Fedora.

Comment 35 Alon Bar-Lev 2014-01-27 14:17:44 UTC

Thank you for your review, it does not seems that ovirt-engine is progressing into inclusion in fedora so for now there is no reason to include this package as well.