https://bugzilla.redhat.com/show_bug.cgi?id=2305882 --- Comment #42 from Kalev Lember <klember@xxxxxxxxxx> --- (In reply to Fabio Valentini from comment #41) > In general, the package looks good to me. Just some minor things that I'd > like to have clarified and / or fixed: > > > https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval > > This is not really applicable - it's not a removal, this package is just not > going to be built for i686 from the start. > You wouldn't need to document or justify this ExcludeArch at all, but it > doesn't hurt to include the link, I guess. I think I'd slightly prefer to have some kind of justification in case someone reads the spec file and wonders what is going on, but I'd be OK to removing it as well if you feel strongly about it. > > # Filter out soname provides for plugins > > %global __provides_exclude_from ^%{_libdir}/papers/.*\\.so$ > > Please put this at the top of the spec file. Putting %globals in the middle > of the spec file makes them really hard to find, and it might even get lost > (i.e. become part of the %description, if it were just a few lines later). Sure, let me move it, good idea. > > -Dintrospection=disabled > > I seem to recall that some Rust-written tool is missing to get this enabled? > Is this on your roadmap, or should we put that on the Rust SIG wishlist? Yes, the tool is called 'gir' and comes from https://github.com/gtk-rs/gir I think it would be good to get it packaged, but I haven't looked at it at all so far. We'll need to enable introspection once something starts using the library through introspection - a python app, for example. For now, I am not aware of anything but I'm sure something is going to appear sooner or later that wants to embed pdf viewing. If I remember right, self tests also depended on introspection, which is why they are disabled with -Dtests=false. Would be nice for it to be on the Rust SIG wishlist! > > %{_libdir}/libppsshell-4.0.so.4{,.*} > > %{_libdir}/libppsdocument-4.0.so.5{,.*} > > %{_libdir}/libppsview-4.0.so.4{,.*} > > Is it intentional that some libraries are in the "main" papers package while > the other two are in the "-libs" subpackage? It's been a while since I did the packaging, but as I recall, I tried to keep the -libs package size minimal and keep the libs that are only for the GUI app in the same subpackage as the GUI app. The idea being that this way, we don't install unnecessary libraries on a system that only ships -thumbnailer and -nautilus and -previewer subpackages. Silverblue for example would ship the app itself as a flatpak, but keep the system integration bits as rpms. I think this might deserve a comment in the spec file; let me add that. > > %{_datadir}/thumbnailers/ > > It looks like nothing owns this directory? > I'm not sure whether I'm using DNF5 correctly, but its repoquery gives me > zero results. > If that is the case, this package will need to co-own the > /usr/share/thumbnailers/ directory. Agreed - let me add the directory. I don't feel like figuring out the repoquery syntax for this right now, but on my F41 system there are a bunch of rpms that all co-own it: $ rpm -qf /usr/share/thumbnailers gdk-pixbuf2-2.42.12-6.fc41.x86_64 gnome-epub-thumbnailer-1.8-1.fc41.x86_64 rsvg-pixbuf-loader-2.59.1-1.fc41.x86_64 libgsf-1.14.53-1.fc41.x86_64 libjxl-0.10.3-5.fc41.x86_64 -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2305882 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202305882%23c42 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
