Spec URL: https://www.bachelot.org/fedora/SPECS/xine-ui.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/xine-ui-0.99.14-1.fc39.src.rpm Description: xine-ui is the traditional, skinned GUI for xine-lib Fedora Account System Username: xavierb
Copr build: https://copr.fedorainfracloud.org/coprs/build/5613863 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2176394-xine-ui/fedora-rawhide-x86_64/05613863-xine-ui/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Taking this review.
Initial spec review: > %{!?_without_caca:BuildRequires: libcaca-devel} > %{!?_without_lirc:BuildRequires: lirc-devel} We can drop the conditional goop here since it's available everywhere xine-lib is (EPEL9 and Fedora) > %if 0%{!?_without_lirc} > export LIRC_CFLAGS="-llirc_client" > export LIRC_LIBS="-llirc_client" > %endif Ditto for here too. > %if 0%{?rhel} && 0%{?rhel} < 8 > %post > # Mime type > update-desktop-database &> /dev/null || : > update-mime-database %{_datadir}/mime &> /dev/null || : > # Icon cache > touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : > > %postun > # Mime type > update-desktop-database &> /dev/null || : > update-mime-database %{_datadir}/mime &> /dev/null || : > # Icon cache > if [ $1 -eq 0 ] ; then > touch --no-create %{_datadir}/icons/hicolor &>/dev/null > gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : > fi > > %posttrans > gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : > %endif We can drop all this since this package can't be built on anything older than EPEL 9. > %{!?_without_caca:%{_bindir}/cacaxine} We can drop the conditional goop here since it's available everywhere xine-lib is (EPEL9 and Fedora)
> It also contains the %{!?_without_caca:color ascii art and} framebuffer version%{!?_without_caca:s}. One more of the conditional goop that can be dropped too.
Thanks Neal, all (and more) fixed: - Use SPDX License: tag - Drop useless conditionals - Drop useless comments - Add EL7 cond for %%post/%%postun - Escape unescaped %% in %%changelog - Drop very old Obsoletes:/Provides: for xine/xine-skins Spec URL: https://www.bachelot.org/fedora/SPECS/xine-ui.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/xine-ui-0.99.14-2.fc39.src.rpm
Created attachment 1949075 [details] The .spec file difference from Copr build 5613863 to 5615144
Copr build: https://copr.fedorainfracloud.org/coprs/build/5615144 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2176394-xine-ui/fedora-rawhide-x86_64/05615144-xine-ui/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Fixed skins URLs. Spec URL: https://www.bachelot.org/fedora/SPECS/xine-ui.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/xine-ui-0.99.14-3.fc39.src.rpm
Created attachment 1949132 [details] The .spec file difference from Copr build 5615144 to 5615532
Copr build: https://copr.fedorainfracloud.org/coprs/build/5615532 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2176394-xine-ui/fedora-rawhide-x86_64/05615532-xine-ui/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
It probably makes sense to drop all the EL7 specific stuff, since we don't have xine-lib in EPEL7 (or even EPEL8).
Sure, but xine-lib still lives in RPM Fusion for EL7 and EL8, so I'd like to keep the conditionals to be able to keep the spec synch'ed across branches.
Okay, fine with me. Everything else looks good here. PACKAGE APPROVED.
Thanks Neal ! Request for unretirement: https://pagure.io/releng/issue/11329
(In reply to Xavier Bachelot from comment #14) > Thanks Neal ! > > Request for unretirement: https://pagure.io/releng/issue/11329 Unretirement was done. You can go ahead and build this.