Bug 516717

rpmlint is done:

$ rpmlint viewnior.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

mock is done:

INFO: Done(viewnior-0.6-1.fc12.src.rpm) Config(fedora-rawhide-i386) 4 minutes 35 seconds

Christoph, is there any deeper reason for the lengthy loop body that fixes the file permissions? Why not simply use "chmod 0644 $file"? But maybe I'm missing something here.

I'm preserving the timestamps. For the stuff inside it's not really important, but for content like docs the original timestamps should be preserved.

See: https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

chmod does not change the time stamps (try it!). You can safely drop the loop.

Drop the conditional
 %if 0%{?_with_gnome:1}
as we don't use these in Fedora.

(In reply to comment #5)
> chmod does not change the time stamps.

Oops, ok (one of these days again...)

(In reply to comment #6)
> Drop the conditional
>  %if 0%{?_with_gnome:1}
> as we don't use these in Fedora. 

We do use similar conditionals and I see no reason why I should drop this. It's an additional feature for people who want to rebuild the package and it allows me to easily change the default behavior.

(In reply to comment #7)
> (In reply to comment #6)
> > Drop the conditional
> >  %if 0%{?_with_gnome:1}
> > as we don't use these in Fedora. 
> 
> We do use similar conditionals and I see no reason why I should drop this. It's
> an additional feature for people who want to rebuild the package and it allows
> me to easily change the default behavior.  

I haven't seen any other packages using conditionals for support of packages. In SuSe it's common behavior and I think that some packages in RPMFusion, too, use them, but not in Fedora. Might you give some examples?

You should enable support for everything in the package in Fedora.

(In reply to comment #8)

> I haven't seen any other packages using conditionals for support of packages.
> In SuSe it's common behavior and I think that some packages in RPMFusion, too,
> use them, but not in Fedora. Might you give some examples?

With "similar" I meant https://fedoraproject.org/wiki/Packaging/DistTag#Conditionals
Another example, not a particulary good one since it is my package is gwget:
http://cvs.fedoraproject.org/viewvc/rpms/gwget/F-11/gwget.spec?view=markup
The epiphany extension was broken and I had to disable it during the F12 mass rebuild. I was able to do this by changing 1 to 0 instead of commenting out large parts of the spec.

> You should enable support for everything in the package in Fedora.

I surely wont do this because I consider the "enable everything" policy one of the biggest mistakes in Fedora. It introduces long dependency chains, just look at totem or pidgim, that require evolution-data-server and all it's dependencies.

Currently we are thinking about viewnior to replace gpicview and become the default image viewer of LXDE. Having a gconf dependency is a no-go there.

(In reply to comment #9)
> http://cvs.fedoraproject.org/viewvc/rpms/gwget/F-11/gwget.spec?view=markup

Wrong link, should be http://cvs.fedoraproject.org/viewvc/rpms/gwget/devel/gwget.spec?revision=1.36&view=markup

Disabling some option on configure is allowed (however
the reason should be commented on the spec file so that
other people can find the reason easily)

Removed the loop and added a comment on the conditional.

http://cwickert.fedorapeople.org/review/viewnior.spec
http://cwickert.fedorapeople.org/review/viewnior-0.6-2.fc12.src.rpm

V 0.7: http://cwickert.fedorapeople.org/review/viewnior-0.7-1.fc12.src.rpm

Well,

* GConf2 dependency
  - If you don't want to remove GConf2 dependency, I guess the
    following line in %description should be removed ( because
    from configure.ac the following feature can be enabled
    only when compiled with GConf2 )
----------------------------------------------------------------
* Set image as wallpaper (only under GNOME)
----------------------------------------------------------------

Other things seem good.
----------------------------------------------------------------
    This package (viewnior) is APPROVED by mtasaka
----------------------------------------------------------------

(In reply to comment #14)
> * GConf2 dependency
>   - If you don't want to remove GConf2 dependency, I guess the

... If you don't want to "add" GConf2 dependency, ...

(In reply to comment #14)
>     I guess the
>     following line in %description should be removed

Good catch. Thanks for the review!

New Package CVS Request
=======================
Package Name: viewnior
Short Description: Elegant Image Viewer
Owners: cwickert
Branches: F-10 F-11
InitialCC:

cvs done.

viewnior-0.7-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/viewnior-0.7-1.fc11

viewnior-0.7-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/viewnior-0.7-1.fc10

Closing.

viewnior-0.7-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

viewnior-0.7-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

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