Bug 518900 - Review Request: desktop-effects - Switch GNOME window management and effects
Summary: Review Request: desktop-effects - Switch GNOME window management and effects
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-08-24 03:40 UTC by Owen Taylor
Modified: 2009-08-24 20:33 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-08-24 20:33:08 UTC
adel.gadllah: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Owen Taylor 2009-08-24 03:40:37 UTC
See https://www.redhat.com/archives/fedora-desktop-list/2009-August/msg00119.html

Spec URL: http://www.gnome.org/~otaylor/desktop-effects.spec
SRPM URL: http://www.gnome.org/~otaylor/desktop-effects-0.8.0-1.src.rpm

Description: 
desktop-effects provides a preference dialog to allow switching the GNOME
desktop between three different window managers: Metacity (the standard
GNOME 2 window manager), Compiz (offering 3D acceleration and special
effects), and GNOME Shell, which offers a preview of the GNOME 3 user
experience.

Notes:

 * Because this is a split-out of the desktop-effects dialog
   previously in compiz-gnome, compiz will need a rebuild to
   require rather than include desktop-effects once this lands.

 * No upstream tarball location - this is pretty much Fedora
   specific and there's no obvious place to put a tarball
   releases. There's a signed tag in git for the release.

Comment 1 Owen Taylor 2009-08-24 03:47:38 UTC
One additional note here is that without the fix I attached to:

 http://bugzilla.gnome.org/show_bug.cgi?id=591171

Switching from gnome-shell to something else may work badly since gnome-shell will try to restart metacity itself when it exits and that will race with what desktop-effects is doing.

(Hopefully we'll have new gnome-shell packages with that fix within the next few days.)

Comment 2 Adel Gadllah 2009-08-24 07:53:55 UTC
(In reply to comment #0)

>  * No upstream tarball location - this is pretty much Fedora
>    specific and there's no obvious place to put a tarball
>    releases. There's a signed tag in git for the release.  

You can upload a tarball at fedorahosted.org see: 

---------------
How can I publish archive releases (tgz, zip, etc) for my project?

Create the archive on your workstation and run scp myProject-0.1.tar.gz fedorahosted.org:<Project Name>. The archive will be located under https://fedorahosted.org/releases/

Is there a more convenient way to access releases than the path /releases/m/y/myproject?

There is. https://fedorahosted.org/released/myproject will go to the same place. The disadvantage of this is you must know the project name and can't browse for projects.
----------------

Build fails in koji:
http://koji.fedoraproject.org/koji/getfile?taskID=1628520&name=build.log
Seems like you need to BR intltool

Besides this the package seems to look good.
* No dist tag, but as it will go away in F13 (so F12 only) it is probably fine.
* You might want to update the Changelog and ship it as %doc

Comment 3 Owen Taylor 2009-08-24 11:47:48 UTC
(In reply to comment #2)
> Create the archive on your workstation and run scp myProject-0.1.tar.gz
> fedorahosted.org:<Project Name>. The archive will be located under
> https://fedorahosted.org/releases/

Oh, cool, I didn't know about that.

> Build fails in koji:
> http://koji.fedoraproject.org/koji/getfile?taskID=1628520&name=build.log
> Seems like you need to BR intltool

Hmm, yep. Also missing desktop-file-utils. (I started a Koji scratch build before, but forgot to check if it finished succesfully. Built fine with those two additions.)
 
> Besides this the package seems to look good.
> * No dist tag, but as it will go away in F13 (so F12 only) it is probably fine.

Just an oversight. Added.

> * You might want to update the Changelog and ship it as %doc  

I don't really like ChangeLogs for projects if there's alraeady a verbose and readable version control log. Adding a disthook to turn git history into a distributed ChangeLog or writing NEWS manually is definitely possible, but for this project I'm too lazy :-)

I've now added a note to the ChangeLog in git that it intentionally isn't being updated.

New spec and SRPM:

Spec URL: http://www.gnome.org/~otaylor/desktop-effects.spec
SRPM URL: http://www.gnome.org/~otaylor/desktop-effects-0.8.0-2.fc12.src.rpm

Thanks for the review!

Comment 4 Adel Gadllah 2009-08-24 12:07:52 UTC
================== REVIEW ==================
[+] source files match upstream: c05c44d0f4b0c1749a54f91eb1257cce4fc38881
[+] package meets naming and versioning guidelines.
[+] specfile is properly named, is cleanly written and uses macros consistently.
[+] dist tag is present.
[+] build root is correct.
[+] license field matches the actual license.
[+] license is open source-compatible: GPLv2+
[+] license text included in package.
[+] latest version is being packaged.
[+] BuildRequires are proper.
[+] compiler flags are appropriate.
[+] %clean is present.
[+] package builds in koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1628777
[+] package installs properly.
[+] debuginfo package looks complete.
[+] rpmlint is silent.
[+] final provides and requires are sane:
	desktop-effects = 0.8.0-2.fc12
	desktop-effects(x86-64) = 0.8.0-2.fc12
	----
	/bin/sh  
	/bin/sh  
	gnome-session  
	hicolor-icon-theme  
	libX11.so.6()(64bit)  
	libXcomposite.so.1()(64bit)  
	libXfixes.so.3()(64bit)  
	libatk-1.0.so.0()(64bit)  
	libc.so.6()(64bit)  
	libc.so.6(GLIBC_2.2.5)(64bit)  
	libcairo.so.2()(64bit)  
	libfontconfig.so.1()(64bit)  
	libfreetype.so.6()(64bit)  
	libgconf-2.so.4()(64bit)  
	libgdk-x11-2.0.so.0()(64bit)  
	libgdk_pixbuf-2.0.so.0()(64bit)  
	libgio-2.0.so.0()(64bit)  
	libglade-2.0.so.0()(64bit)  
	libglib-2.0.so.0()(64bit)  
	libgmodule-2.0.so.0()(64bit)  
	libgobject-2.0.so.0()(64bit)  
	libgtk-x11-2.0.so.0()(64bit)  
	libpango-1.0.so.0()(64bit)  
	libpangocairo-1.0.so.0()(64bit)  
	libpangoft2-1.0.so.0()(64bit)  
	libpthread.so.0()(64bit)  
	libxml2.so.2()(64bit)  
	rpmlib(CompressedFileNames) <= 3.0.4-1
	rpmlib(FileDigests) <= 4.6.0-1
	rpmlib(PayloadFilesHavePrefix) <= 4.0-1
	rtld(GNU_HASH)  
	rpmlib(PayloadIsXz) <= 5.2-1
[+] no shared libraries are added to the regular linker search paths.
[+] owns the directories it creates.
[+] doesn't own any directories it shouldn't.
[+] no duplicates in %files.
[+] file permissions are appropriate.
[+] scriptlets are sane.
[+] code, not content.
[+] documentation is small, so no -docs subpackage is necessary.
[+] %docs are not necessary for the proper functioning of the package.
[+] no headers.
[+] no pkgconfig files.
[+] no libtool .la droppings.
[+] desktop files valid and installed properly.
================== COMMENTS ==================

Looks fine => APPROVED

Comment 5 Owen Taylor 2009-08-24 12:55:52 UTC
New Package CVS Request
=======================
Package Name: desktop-effects
Short Description: Switch GNOME window management and effects
Owners: otaylor drago01
Branches: 
InitialCC:

Comment 6 Kevin Fenzi 2009-08-24 19:41:32 UTC
cvs done.

Comment 7 Owen Taylor 2009-08-24 20:33:08 UTC
Build into F12


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