Spec URL: http://cwickert.fedorapeople.org/review/viewnior.spec SRPM URL: http://cwickert.fedorapeople.org/review/viewnior-0.6-1.fc12.src.rpm Description: Viewnior is an image viewer program. Created to be simple, fast and elegant. It's minimalistic interface provides more screenspace for your images. Among its features are: * Fullscreen & Slideshow * Rotate, flip, save, delete images * Animation support * Browse only selected images * Navigation window * Set image as wallpaper (only under GNOME) * Simple interface * Configurable mouse actions
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. ***