SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05598376-input-leap/input-leap.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05598376-input-leap/input-leap-2.4.0-0.1.20230306gite71fbbfe.fc39.src.rpm Description: Input Leap is software that mimics the functionality of a KVM switch, which historically would allow you to use a single keyboard and mouse to control multiple computers by physically turning a dial on the box to switch the machine you're controlling at any given moment.
Taking this review.
I did some minor changes :) SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05602818-input-leap/input-leap.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05602818-input-leap/input-leap-2.4.0-0.1.20230306gite71fbbfe.fc39.src.rpm
Created attachment 1948668 [details] The .spec file difference from Copr build 5598422 to 5602834
Initial spec review: > Version: 2.4.0 > %if %{with_snapshot} > Release: %autorelease -p -s %{gitdate}git%{shortcommit} > %else > Release: %autorelease Please move the snapshot versioning to "Version", like so: "2.4.0%{?with_snapshot:^%{gitdate}git%{shortcommit}}" Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots > -DINPUTLEAP_BUILD_TESTS=OFF \ > -DINPUTLEAP_USE_EXTERNAL_GTEST=false We do have gtest-devel for this... > desktop-file-install --delete-original \ > --dir %{buildroot}%{_datadir}/applications \ > --set-icon=%{_datadir}/icons/hicolor/scalable/apps/%{name}.svg \ > %{buildroot}%{_datadir}/applications/%{name}.desktop I looked at the original desktop file, and I don't see a reason to rewrite the file? https://github.com/input-leap/input-leap/blob/master/res/input-leap.desktop Can we just replace this with desktop-file-validate in %check? Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_file_install_usage We also want validation of the AppStream metainfo file. Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
Shouldn't this package *replace* barrier with something like Obsoletes?
(In reply to Brian J. Murrell from comment #5) > Shouldn't this package *replace* barrier with something like Obsoletes? Just to clarify why... but my understanding is that upstream barrier's development has all moved to input-leap and thus barrier is effectively abandoned and accordingly, packaging effort for barrier has also ceased with the packaging effort for this package taking over where that stopped. That seems to me to be exactly what Obsoletes was created for. It means people will have their barrier upgraded to input-leap once the package is in the repo without people needing to know that they would otherwise have to manually remove barrier and manually install input-leap. I would guess that most people are likely never going to realize that barrier is effectively dead and they this manual upgrade step is necessary.
Spec file has changed as Neal requested. SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05603716-input-leap/input-leap.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05603716-input-leap/input-leap-2.4.0%5E20230306gite71fbbfe-1.fc39.src.rpm
Still no Obsoletes and no comment about it either way.
Created attachment 1948799 [details] The .spec file difference from Copr build 5602834 to 5603815
Copr build: https://copr.fedorainfracloud.org/coprs/build/5603815 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2175818-input-leap/fedora-rawhide-x86_64/05603815-input-leap/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.
Hello Brian, can we discuss this after the package approval ?
Hi Ali. It seems like adding Obsoletes: would be part of the package approval process (it's going to modify the specfile after all) and discussing it afterward would be like discussing how to get the toothpaste out of the tube after it's already all been squeezed out. Or am I misunderstanding something?
No, the package replacement needs to be part of this package review, since it is intended to replace barrier and synergy.
That means we need "Obsoletes+Provides" stanza for barrier (and potentially synergy too...)
Ahh. Good catch on replacing synergy as well Neal.
Ok, we can obsolete barrier (primary developers moved to inputleap) but not sure about synergy. https://github.com/symless/synergy-core/commits/master https://src.fedoraproject.org/rpms/synergy/commits/rawhide
Heh, yes. I tend to forget that synergy is dead to me, but not everyone. So I agree that probably only Obsoletes+Provides for barrier is appropriate.
SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05615430-input-leap/input-leap.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05615430-input-leap/input-leap-2.4.0%5E20230306gite71fbbfe-1.fc39.src.rpm
Created attachment 1949127 [details] The .spec file difference from Copr build 5603815 to 5615441
Copr build: https://copr.fedorainfracloud.org/coprs/build/5615441 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2175818-input-leap/fedora-rawhide-x86_64/05615441-input-leap/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.
> %if %{with_snapshot} > Version: 2.4.0%{?with_snapshot:^%{gitdate}git%{shortcommit}} > %else > Version: 2.4.0 > %endif You can drop the %if conditional. it's redundant.
I've missed that. SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05619358-input-leap/input-leap.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05619358-input-leap/input-leap-2.4.0%5E20230306gite71fbbfe-1.fc39.src.rpm
Created attachment 1949414 [details] The .spec file difference from Copr build 5615441 to 5619428
Copr build: https://copr.fedorainfracloud.org/coprs/build/5619428 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2175818-input-leap/fedora-rawhide-x86_64/05619428-input-leap/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.
> %cmake . \ You can drop the ".", as it's handled by the %cmake macro now.
SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05651699-input-leap/input-leap.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/05651699-input-leap/input-leap-2.4.0%5E20230306gite71fbbfe-1.fc39.src.rpm
Created attachment 1951302 [details] The .spec file difference from Copr build 5619428 to 5651771
Copr build: https://copr.fedorainfracloud.org/coprs/build/5651771 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2175818-input-leap/fedora-rawhide-x86_64/05651771-input-leap/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.
Things look good now! PACKAGE APPROVED.
The Pagure repository was created at https://src.fedoraproject.org/rpms/input-leap
Thank you guys :)
I do have a COPR project building this for F37->rawhide if anyone wants early access on current-release Fedora. https://copr.fedorainfracloud.org/coprs/brianjmurrell/input-leap/ dnf copr enable brianjmurrell/input-leap I've installed it. Won't be able to test it until tomorrow when I go to work.