https://bugzilla.redhat.com/show_bug.cgi?id=1998270



--- Comment #11 from Kalev Lember <klem...@redhat.com> ---
> Version:        %{majorver}.%{minorver}.%{patchver}

This is not a review blocker, but any chance you could just spell it out as
"Version: 5.2.0"? We have some automation scripts that I use for GNOME package
mass updates that would break if it's split up like this. I am the one who's
been handling the GNOME mass updates recently but amigadave is going to take
over for the next cycle.

I would keep majorver as a global though, but maybe rename it to api_ver or
something like that, as that would make it easier to bump from gtksourceview 5
to gtksourceview 6 in the future. What do you think?


> %{_libdir}/girepository-1.0/GtkSource-%{majorver}.typelib

The directory ownership here is incorrect. You need to make sure that
%{_libdir}/girepository-1.0 directory itself is included in the package's list
of files: otherwise when the package is removed it leaves behind the empty
directory. You could do it in two ways, either explicitly listing the
directory:

%dir %{_libdir}/girepository-1.0
%{_libdir}/girepository-1.0/GtkSource-%{majorver}.typelib

or recursively list both the directory and its contents:

%{_libdir}/girepository-1.0/

Which one of these to use is just a style question.

This and other gir directories might get added to a filesystem package in the
future (there's an open FPC ticket about this), but as of right now we need to
be explicit to make sure they are listed correctly.


> %{_datadir}/gir-1.0/GtkSource-%{majorver}.gir

Same thing here: Need to make sure %{_datadir}/gir-1.0 directory is listed in
the files list.


> %{_datadir}/gtk-doc/html/*

... and here: %{_datadir}/gtk-doc and %{_datadir}/gtk-doc/html directories need
to be listed in the files list.


Beyond these nitpicks, it all looks nice and clean to me -- thanks for
packaging it up! I don't see any other issues and it's just a new
parallel-installable version of the existing gtksourceview4 package so I'd be
happy to review+ this if you can fix the nits above.

Do you need a sponsor for the packaging group, by the way? I can help with that
if you need it.


-- 
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=1998270
_______________________________________________
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to