Bug 458030

just a quick initial review from just looking at the spec file:

- > BuildRequires: cmake >= 2.4.5, qt4-devel >= 4.3.0, eigen-devel >= 1.0.5, openbabel-devel >= 2.2.0, python-devel >= 2.5.1

 Thats a pretty long line; would look much cleaer if you split it

- > Requires: libavogadro = %{version}

 Are you sure that's needed? RPM will likely add a dep on the libs it needs automatically

- the description is quite small (just 55 chars instead off less then 80); no blocker, but looks odd

- > %package -n libavogadro

 I'd say the libs package should be called avogadro-libs -- that's the usual way to name the libs subpackage  in Fedora 

- > # set permissions

 Please add a comment to the spec file why you do this

I have uploaded a new version, which should fix your points and renamed the shared library package to avogadro-libs.

Spec URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuilds/avogardo-0.8.1-2.fc9.src.rpm

Here is a new version, which contains a reorganized spec file and fixes some things concerning the icon implementation.

Spec URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro-0.8.1-3.fc9.src.rpm

...and another new version fixing some typos:

Spec URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro-0.8.1-4.fc9.src.rpm

= general =

- rpmlint mentions
 >  avogadro.src: W: strange-permission avogadro-0.8.1.tar.bz2 0777
 please fix

 > avogadro-libs.x86_64: W: no-documentation
 > avogadro-devel.x86_64: W: no-documentation
 Can be ignored


- doesn't build in mock (and thus will faill in the Fedora buildsys):

 > /var/tmp/rpm-tmp.5185: line 37: desktop-file-install: command not found


- from mock right after the first cmake call:

 > -- docbook tools not found, doc targets disabled

Please check; docs should be shipped normally. Likely needs just a porper BuildRequire as well


== libs subpackage ==

- Group should be "System Environment/Libraries"

== devel subpackage ==

- Change 
 > Requires:       %{name} = %{version}
to
 > Requires:       %{name}-libs = %{version}-%{release} 

== More Comments ==

Looks good otherwise. Once the above things are fixed I'll approve the package and sponser you.

Here's a version, which should address your points:

Spec URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro-0.8.1-5.fc9.src.rpm

Thanks!

...and yet another version adding the handbook to the docs:

Spec URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuilds/avogadro-0.8.1-6.fc9.src.rpm

Package looks good now; APPROVED; please apply to the packager group in FAS

@Thorsten

I have updated the review flag - you might have forgotten :)

New Package CVS Request
=======================
Package Name: avogadro
Short Description: Avogadro is an advanced Molecular editor
Owners: sdz
Branches: F-8 F-9
InitialCC:

cvs done

avogadro-0.8.1-6.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/avogadro-0.8.1-6.fc9

avogadro-0.8.1-6.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update avogadro'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-8291

avogadro-0.8.1-6.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.