Bug 675726 - Review Request: cdm - Very minimalistic display manager
Summary: Review Request: cdm - Very minimalistic display manager
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-02-07 14:37 UTC by Mikhail Kulemin
Modified: 2011-03-05 22:59 UTC (History)
4 users (show)

Fixed In Version: cdm-0.5.3-4.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-02-25 12:03:04 UTC
Type: ---
Embargoed:
lemenkov: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)
New spec file for build 2 (1.46 KB, application/octet-stream)
2011-02-07 15:25 UTC, Mikhail Kulemin
no flags Details
New spec v3 (1.36 KB, application/octet-stream)
2011-02-08 14:12 UTC, Mikhail Kulemin
no flags Details
spec file v4 (1.65 KB, application/octet-stream)
2011-02-22 20:09 UTC, Mikhail Kulemin
no flags Details

Description Mikhail Kulemin 2011-02-07 14:37:00 UTC
Spec URL: http://landgraf.fedorapeople.org/cdm.spec

SRPM URL: https://koji.fedoraproject.org/koji/getfile?taskID=2767054&name=cdm-0.5.3-1.fc14.src.rpm

Description: Small login manager. Written in pure bash, CDM has no other dependencies, yet supports multiple users/sessions and can start virtually any DE/WM. CDM can replace slim in ultra small environments.

Koji build ok.

Comment 1 Pavel Zhukov 2011-02-07 14:57:08 UTC
few comments:

-- you should NOT point bash to Requires: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

-- if you don't plan package for EPEL you should not point BuildRoot and clean sections; 

-- you SHOULD point full URL to Source (vendor doen't prohibit it) http://cdm.ghost1227.com/repo/cdm-latest.tar.gz or http://cdm.ghost1227.com/repo/cdm-0.5.3.tar.gz

-- you SHOULD include README (create it) and COPYING  (license text  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text) to your package. You can include CHANGELOG too. 





-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 2 Mikhail Kulemin 2011-02-07 15:24:05 UTC
Fixed 
Is README file neeeded?

New srpm
https://koji.fedoraproject.org/koji/getfile?taskID=2767219&name=cdm-0.5.3-2.fc14.src.rpm

Comment 3 Mikhail Kulemin 2011-02-07 15:25:35 UTC
Created attachment 477430 [details]
New spec file for build 2

Comment 4 Peter Lemenkov 2011-02-08 11:28:50 UTC
I'll review it

Comment 5 Peter Lemenkov 2011-02-08 11:39:55 UTC
Unblocking FE-NEEDSPONSOR - I just sponsored Mikhail.

Comment 6 Peter Lemenkov 2011-02-08 12:42:53 UTC
Few notes before review:

* Please, drop commented out lines in the %install section

* Drop the following line as well

install -Dm644 src/xinitrc.skel $RPM_BUILD_ROOT/%{_docdir}/%{name}-%{version}/xinitrc.example

this file should be installed just as other docs (mark it as %doc in the %files section).

* The directory %{_datarootdir}/%{name}/ left unowned. Please apply the following change:

- %{_datarootdir}/%{name}/*
+ %{_datarootdir}/%{name}/

Note - trailing asterisk was removed.

Comment 7 Mikhail Kulemin 2011-02-08 14:12:15 UTC
Created attachment 477617 [details]
New spec v3

Comment 8 Mikhail Kulemin 2011-02-08 14:13:02 UTC
Fixed

https://koji.fedoraproject.org/koji/getfile?taskID=2784901&name=cdm-0.5.3-3.fc14.src.rpm

spec in attachments

Comment 9 Peter Lemenkov 2011-02-08 15:03:53 UTC
Ok, few more notes:

* xinitrc must be installed into %{_datarootdir}/%{name}/ .
* Missing requires - dialog, xorg-x11-utils, dbus, xorg-x11-xinit
* cdmrc mentions 'awesome' as one of the window managers. Please change it to some more Fedora-specific.
* Licensing terms are misleading - the LICENSE file contains GPLv3, but the main script, ./src/cdm, explicitly menions GPLv2+. Please, contact upstream for clarification.

Comment 10 Mikhail Kulemin 2011-02-22 20:08:11 UTC
Fixed this issues.
Add ConsoleKit in requires list (it is used with dbus)
After discussion license issue was fixed - right license is GPLv2+ and now upstream sources have correct license text in tarbal.

SRPM URL: https://koji.fedoraproject.org/koji/getfile?taskID=2857478&name=cdm-0.5.3-4.fc14.src.rpm

spec file in attachment

Comment 11 Mikhail Kulemin 2011-02-22 20:09:51 UTC
Created attachment 480233 [details]
spec file v4

Comment 12 Peter Lemenkov 2011-02-24 10:22:44 UTC
REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is NOT silent but all its messages can be omitted in this case:

sulaco ~/rpmbuild/SPECS: rpmlint ../RPMS/noarch/cdm-0.5.3-4.fc15.noarch.rpm 
cdm.noarch: W: spelling-error Summary(en_US) minimalistic -> minimalist, minimalism, materialistic
cdm.noarch: W: spelling-error Summary(en_US) login -> loin, logic, lo gin
cdm.noarch: W: spelling-error %description -l en_US minimalistic -> minimalist, minimalism, materialistic
cdm.noarch: W: spelling-error %description -l en_US login -> loin, logic, lo gin
cdm.noarch: W: spelling-error %description -l en_US kdm -> km, Adm, Kim
cdm.noarch: W: spelling-error %description -l en_US gdm -> gm, gem, gum
cdm.noarch: W: spelling-error %description -l en_US qingy -> dingy, mingy, zingy

^^^ False positive.

cdm.noarch: W: non-conffile-in-etc /etc/profile.d/zzz-cdm-profile.sh

^^^ That's ok - this file is not intented to be changed by end-=user.

cdm.noarch: W: no-manual-page-for-binary cdm

^^^ Unfortunately this binary doesn't have corresponding man-page. So no luck for us here too.

1 packages and 0 specfiles checked; 0 errors, 9 warnings.
sulaco ~/rpmbuild/SPECS:

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (GPLv2 or later).
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum cdm-0.5.3.tar.gz*
fb9ada13d3416305828c99943698fe7df3b0ab91bd6099e7b2707e1d8dd99a23  cdm-0.5.3.tar.gz
fb9ada13d3416305828c99943698fe7df3b0ab91bd6099e7b2707e1d8dd99a23  cdm-0.5.3.tar.gz.1
sulaco ~/rpmbuild/SOURCES:

+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files.
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
0 Apparently, this packages is a GUI application, but we don't need a *.desktop file for starting it from other desktops.it. It is a desktop itself.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Ok, I can't find any other issues, so this package is

APPROVED.

Comment 13 Mikhail Kulemin 2011-02-24 11:13:06 UTC
New Package SCM Request
=======================
Package Name: cdm
Short Description: Very minimalistic login manager
Owners: mkulemin
Branches: f14 f15
InitialCC:

Comment 14 Jason Tibbitts 2011-02-24 18:28:35 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2011-02-25 09:54:59 UTC
cdm-0.5.3-4.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/cdm-0.5.3-4.fc14

Comment 16 Fedora Update System 2011-02-25 10:03:33 UTC
cdm-0.5.3-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/cdm-0.5.3-4.fc15

Comment 17 Fedora Update System 2011-03-03 02:54:53 UTC
cdm-0.5.3-4.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2011-03-05 22:59:19 UTC
cdm-0.5.3-4.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.