Product: Fedora Version: rawhide Component: Package Review Artem <ego.cordatus@xxxxxxxxx> has canceled Package Review <package-review@xxxxxxxxxxxxxxxxxxxxxxx>'s request for Artem <ego.cordatus@xxxxxxxxx>'s needinfo: Bug 1751216: Review Request: xlunch - Graphical app launcher for X with minimal dependencies https://bugzilla.redhat.com/show_bug.cgi?id=1751216 --- Comment #7 from Artem <ego.cordatus@xxxxxxxxx> --- (In reply to Artur Frenszek-Iwicki from comment #4) > Fedora ships OpenSans (open-sans-fonts), why change this? Because of consistency and Cantarell more system-widely used also canonical for GNOME DE. Just wanted better defaults. But if you insist i'll accept this easy since and we can keep upstream defaults here. > Also, if "otherwise it won't run", then the font package should go in > Requires:, not Recommends:. Good point. Now 'abattis-cantarell-fonts' hard dep and there should'n be any issue since it availablin even in CentOS7 http://mirror.centos.org/centos/7/os/x86_64/Packages/abattis-cantarell-fonts-0. 0.25-1.el7.noarch.rpm > ># "Error generating new entries"" for key "Exec" in group "Desktop Entry" contains a reserved character '$' outside of a quote > ># error: required key "Type" in group "Desktop Entry" is not present > ># https://github.com/Tomas-M/xlunch/issues/109 > >#desktop-file-validate %%{buildroot}%%{_datadir}/applications/genentries.desktop > I looked at the .desktop file and I think the best way to solve this would > be to extract the whole chain of commands into a helper shell script, and > then just launch that script from the .desktop file. I though this as well, but TBH i've lost interest a little bit to this package since i've packaged Ulauncher and sgtk-menu for Fedora. But i fixed that and upstreamed, see links in SPEC file. > Also, taking a look at the upstream Makefile - the files are installed using > "cp". Please consider patching this (and possibly sending the patch > upstream) to use "cp -p" so that file timestamps are preserved. Ok, this is in TODO area. (In reply to Robert-André Mauchin 🐧 from comment #6) > - Is there files in this? That you would need to %config(noreplace)? > > %{_sysconfdir}/%{name} Yep. Fixed. Also this a directory, so now this look like this: %config(noreplace) %{_sysconfdir}/%{name}/ --- https://download.copr.fedorainfracloud.org/results/atim/xlunch/fedora-33-x86_64 /01695721-xlunch/xlunch.spec https://download.copr.fedorainfracloud.org/results/atim/xlunch/fedora-33-x86_64 /01695721-xlunch/xlunch-4.5.4-1.fc33.src.rpm There is still some issues (new) but it works. _______________________________________________ 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
