Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=924310 Alex G. <mr.nuke.me@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mr.nuke.me@xxxxxxxxx Assignee|nobody@xxxxxxxxxxxxxxxxx |mr.nuke.me@xxxxxxxxx Flags| |fedora-review? --- Comment #5 from Alex G. <mr.nuke.me@xxxxxxxxx> --- (Disclaimer: not an official review) > %files dvi > %{_libdir}/atril/3/backends/libdvidocument.so* > > %files djvu > %{_libdir}/atril/3/backends/libdjvudocument.so > > %files xps > %{_libdir}/atril/3/backends/libxpsdocument.so Why the "*" after libdvidocument.so, but not after the others? All of the libraries are unversioned, so removing the "*" will give you a heads up if upstream decides to version those libraries. > %package libs > %package dvi > %package djvu > %package xps Is there a reason to to separate the backends from the libs package? Think of a user installing mate-document-viewer. Will they be able to view $backendname files without installing the $backendname subpackage? I see this is a fork of evince, and evince uses the same splitup. However, unless there is a good reason to do so, I think keeping everything in -libs is saner. (Just look at texlive, which has almost 1000 subpackages). > sed -i -e 's,Categories=MATE;GTK;Graphics;VectorGraphics;Viewer;,Categories=GTK;Graphics;VectorGraphics;Viewer;,g' data/atril.desktop.in.in data/atril.desktop.in.in > sed -i -e '/GTK;Graphics;VectorGraphics;Viewer;/ a\OnlyShowIn=MATE;' data/atril.desktop.in.in Small comment explaining why this is needed. >%configure \ > --disable-static \ > --disable-scrollkeeper \ Whitespace issue. either use only tabs, or only spaces, but don't mix them up. There are a few more issues. I'll post a detailed review later. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=aQAaUTGv4M&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review