Correction:
Spec URL: https://jdulaney.fedorapeople.org/qtile.spec
SRPM URL:  https://jdulaney.fedorapeople.org/qtile-0.9.0-1.fc22.src.rpm

Informal comment:

No need for desktop-file-validate to be used here, that's only for GUI items to appear in menus, e.g. packages that own .desktop content under /usr/share/applications

Second correction because I fail:
Spec URL: https://jdulaney.fedorapeople.org/qtile.spec
SRPM URL:  https://jdulaney.fedorapeople.org/qtile-0.9.0-2.fc22.src.rpm

Taking this ^^

*** Bug 1131825 has been marked as a duplicate of this bug. ***

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

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


Issues:
=======
- Package uses either %{buildroot} or $RPM_BUILD_ROOT
  Note: Using both %{buildroot} and $RPM_BUILD_ROOT
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros

  ---> Use either one or the other…  ;)


===== 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.
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "MIT/X11 (BSD like)", "Apache (v2.0)", "GPL (v3 or later)", "Unknown or
     generated". 137 files have unknown license. Detailed output of
     licensecheck in
     /home/besser82/shared/fedora/review/1188093-qtile/licensecheck.txt

     ---> Package is not MIT-only…  Please provide proper license-
          breakdown in spec-file.  Lecensecheck.txt is attached.

[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/lib/python2.7/site-
     packages/qtile-0.9.0-py2.7.egg-info, /usr/lib/python2.7/site-
     packages/libqtile
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/python2.7/site-
     packages/libqtile, /usr/lib/python2.7/site-packages/qtile-0.9.0-py2.7
     .egg-info

     ---> Please change the %files-section in spec-file to fix this:
          -%{python2_sitelib}/qtile-%{version}-py2.7.egg-info/*
          -%{python2_sitelib}/libqtile/*
          +%{python2_sitelib}/qtile-%{version}-py%{python2_version}.egg-info
          +%{python2_sitelib}/libqtile

[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[!]: Package consistently uses macros (instead of hard-coded directory names).

     ---> "py2.7.egg-info" vs. "py%{python2_version}.egg-info"

[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]: 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 10240 bytes in 2 files.
[!]: Package complies to the Packaging Guidelines

     ---> Severe errors / issues are present…  ;(

[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 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 does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or desktop-
     file-validate if there is such a file.
[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

Python:
[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
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep


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

Generic:
[!]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.

     ---> There are LICENSE-files missing for the mixed-up
          licensing in sources.

[!]: Final provides and requires are sane (see attachments).

     ---> Package requires /usr/bin/env.  Wrong hash-bang?

[?]: Package functions as described.

     ---> not tested, yet;  packaging-issues present.

[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[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.

     ---> There is test-suite provided by the package, but not run
          during %check.

[!]: Packages should try to preserve timestamps of original installed files.

     ---> use `install -pm` instead of simple `install -m`.

[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]: Buildroot is not present
[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]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.


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

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: qtile-0.9.0-2.fc22.noarch.rpm
          qtile-0.9.0-2.fc22.src.rpm
qtile.noarch: W: spelling-error %description -l en_US scriptability -> script ability, script-ability, inscrutability
qtile.noarch: W: spelling-error %description -l en_US workspaces -> work spaces, work-spaces, works paces
qtile.src: W: spelling-error %description -l en_US scriptability -> script ability, script-ability, inscrutability
qtile.src: W: spelling-error %description -l en_US workspaces -> work spaces, work-spaces, works paces

--->  Please double-check spelling.

qtile.noarch: W: no-manual-page-for-binary qtile-session
qtile.noarch: W: no-manual-page-for-binary qtile-run

---> Ignored.

qtile.src: W: file-size-mismatch v0.9.0.tar.gz = 280935, https://github.com/qtile/qtile/archive/v0.9.0.tar.gz = 280182

---> dafuq?  Please remove the tarball in your SOURCES-dir and
     re-download it using `spectool -g -R ./qtile.spec`.

2 packages and 0 specfiles checked; 0 errors, 7 warnings.


Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:

---> Possibly a bug in rpmlint…  :/


Requires
--------
qtile (rpmlib, GLIBC filtered):
    /usr/bin/env
    /usr/bin/python2
    python(abi)
    python-cairocffi
    python-cffi
    python-trollius
    python-xcffib


Provides
--------
qtile:
    qtile


Source checksums
----------------
https://github.com/qtile/qtile/archive/v0.9.0.tar.gz :
  CHECKSUM(SHA256) this package     : c92c089ede32643a828b19fecda1184ae21d2ec0007cbea37a2ede575ce8cc34
  CHECKSUM(SHA256) upstream package : a49930358b282085afe6f84800a01f7495009da4024d46a0ddceb0f287e0aed6
However, diff -r shows no differences


Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1188093
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG


===== Additional remarks =====

  * Installation of man-pages in %files
    -%{_mandir}/man1/qsh.1.gz
    -%{_mandir}/man1/qtile.1.gz
    +%{_mandir}/man1/qsh.1.*
    +%{_mandir}/man1/qtile.1.

    ---> Compression might change on further releases.  Avoid FTBFS…

  * Usage of %setup:
    -%setup -q -n qtile-%{version}
    +%setup -q -n qtile-%{version}

    ---> %setup defaults to -n %{name}-%{version} anyways.


===== Solution =====

NOT approved.  Please fix those issues and I'll have another look.

Created attachment 988980 [details]
licensescheck.txt from fedora-review

qtile.noarch: W: no-manual-page-for-binary qtile-session
qtile.noarch: W: no-manual-page-for-binary qtile-run

---> Ignored.  

Hint: You can use help2man to generate some nice manpages if there's some useful output for the --help or -h parameter option. But that is not a must. Some samples could be found in my packages ;)
https://fedorahosted.org/fpc/ticket/486

(In reply to Raphael Groner from comment #8)
> qtile.noarch: W: no-manual-page-for-binary qtile-session
> qtile.noarch: W: no-manual-page-for-binary qtile-run
> 
> ---> Ignored.  
> 
> Hint: You can use help2man to generate some nice manpages if there's some
> useful output for the --help or -h parameter option. But that is not a must.
> Some samples could be found in my packages ;)
> https://fedorahosted.org/fpc/ticket/486

I intentionally ignored the absence of man-pages in this review run…  There are real errors to fix in the spec-file.  Creating man-pages with `help2man` is bonus-score, when everything else is fine and compliant to the guidelines.

John, please add me as comaintainer, my FAS ID is cicku. I submitted this initially but I didn't read the email so I failed to update it, it's disappointing to see the wipe out.

Am working with upstream to get things cleaned up.  For some reason, fedora-review keeps blowing up on my local system, dunno why.

Christopher, aye, I'll add you as co-maintainer.  Do you want to help maintain the deps I maintain, as well?

(In reply to John Dulaney from comment #11)
> Am working with upstream to get things cleaned up.  For some reason,
> fedora-review keeps blowing up on my local system, dunno why.
> 
> Christopher, aye, I'll add you as co-maintainer.  Do you want to help
> maintain the deps I maintain, as well?

Sure, just add me and I will keep an eye on them when I have time.

Thanks!

Spec URL: https://jdulaney.fedorapeople.org/qtile-0.9.1-1.fc22.src.rpm
SRPM URL: https://jdulaney.fedorapeople.org/qtile.spec


Working with upstream on issues, also fixed my own mistakes in the .spec file.  I think we're ready for another go.  Note version bump for upstream fixes.

I should mention that a local mockbuild may fail until the updated python-xcffib makes it through mash.  Accordingly, I did a koji scratch build; the result may be found here:  https://jdulaney.fedorapeople.org/qtile-0.9.1-1.fc22.noarch.rpm

Beuler?

Taking liberty of envoking a rushed stalled reviewer process a bit, since this package review needs to be done by impending f22 feature freeze.

As mentioned:

1.  MUST fix Licensing: NOT ok

In addition to MIT licensed stuff, we have
libqtile/widget/pacman.py:GPL (v3 or later)
libqtile/widget/google_calendar.py:Apache (v2.0)

So, I'd suggest using something like:
# All MIT except for:
# libqtile/widget/pacman.py:GPL (v3 or later)
# libqtile/widget/google_calendar.py:Apache (v2.0)
License: MIT and GPLv3+ and ASL 2.0


2. SHOULD replace
%files
...
%{_mandir}/man1/qsh.1.gz
%{_mandir}/man1/qtile.1.gz
with
%{_mandir}/man1/qsh.1*
%{_mandir}/man1/qtile.1*


3.  SHOULD simplify %files
I *think* there's some redundancy,
%{python2_sitelib}/qtile-%{version}-py%{python2_version}.egg-info
%{python2_sitelib}/qtile-%{version}-py%{python2_version}.egg-info/*
%{python2_sitelib}/libqtile
%{python2_sitelib}/libqtile/*
can be replaced with
%{python2_sitelib}/qtile-%{version}-py%{python2_version}.egg-info/
%{python2_sitelib}/libqtile/
(including directories implies including everything recursively under it too)


So, all I see is 1 MUST blocker (so far), the licensing.  fix that, and we should be good.

Spec URL: https://jdulaney.fedorapeople.org/qtile-0.9.1-1.fc22.src.rpm
SRPM URL: https://jdulaney.fedorapeople.org/qtile.spec

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 must be included in %license. 
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

Means: Replace the line

%doc LICENSE

with

%license LICENSE

ah, that is a recent guidelines change I'd forgotten about.  I won't treat it as a blocker, but please do update to use %license tag prior to doing any builds.

2 other minor things:

Please do include the comment:
# All MIT except for:
# libqtile/widget/pacman.py:GPL (v3 or later)
# libqtile/widget/google_calendar.py:Apache (v2.0)
near the License: tag, to explain what it's all about

you should add a %changelog (and ideally bump Release: tag) entry every time you update things (even for reviews).


Otherwise, APPROVED

> No need for desktop-file-validate to be used here, that's only
> for GUI items to appear in menus, e.g. packages that own .desktop
> content under /usr/share/applications

The guidelines should be updated then to be more specific about such an exception:

  https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files

Installing only valid .desktop files in /usr/share/xsessions would certainly be a good thing.

The guideline does say 
* "If a package contains a GUI application" which this arguably is not
and
* references the desktop-entry-spec, which includes as first sentence of Introduction: "Both the KDE and GNOME desktop environments have adopted a similar format for "desktop entries", or configuration files describing how a particular program is to be launched, how it appears in menus, etc".  Again, this case is clearly not a program to be launched from a desktop environment.

Sure, that may not be obvious at first glance.  I'll see about poking fpc to update that to be clearer somehow (suggestions welcome).

Maybe enough to say something like this?

This guideline applies to .desktop files installed under /usr/share/applications

or a little more verbose version:

This guidelines applies to .desktop files installed under $XDG_DATA_DIRS/applications (where XDG_DATA_DIRS defaults to /usr/local/share:/usr/share if undefined), see also:
http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

Spec URL: https://jdulaney.fedorapeople.org/qtile-0.9.1-2.fc22.src.rpm
SRPM URL: https://jdulaney.fedorapeople.org/qtile.spec

New Package SCM Request
=======================
Package Name: qtile
Short Description: A pure-Python tiling window manager
Upstream URL: http://qtile.org
Owners: jdulaney
Branches: f22
InitialCC:

> I'll see about poking fpc to update that to be clearer somehow
> (suggestions welcome).

The current wording likely has been inherited from very old guidelines that only defined a primary goal, i.e. make packagers aware of the desktop-file tools related GUI programs where we wanted to validate and/or add an installed .desktop file as to avoid ending up with a missing menu entry.

Openbox is a GUI program, too. One could even call it "application", and that's also its "Type=" in the .desktop file. However, it does more than drawing a single window to run within. Nevertheless, do such .desktop files, which are handled by display managers, follow the Desktop Entry Specification or not? Imagine Openbox installed a .desktop file that would not be recognized by a display manager. That would be even worse than a GUI program missing in "a menu".

Git done (by process-git-requests).

Please provide also a package for F21. It would be nice to see qtile there, too.

Unfortunately, not quite doable unless I package up an older version.  The current release requires dependencies outside my control that are in F22 but not F21.  Also, I don't have a spec file for older qtile any more.

pkgDB ACL requested.