https://bugzilla.redhat.com/show_bug.cgi?id=2107381 --- Comment #1 from Carl George 🤠 <carl@xxxxxxxxxx> --- The Source URL should use the format in the package guidelines. Optionally consider using Source instead of Source0, and taking advantage of the %{url} macro. -Source0: https://github.com/aristocratos/btop/archive/refs/tags/v%{version}.tar.gz +Source: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags ================================================================================ Does this need both gcc and gcc-c++ to build? I only see CXX in the Makefile, not CC, so I think you can remove the former. -BuildRequires: gcc ================================================================================ Wrap the description at 80 characters. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_description ================================================================================ Consider including the CHANGELOG.md file as a %doc in %files. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation ================================================================================ You'll need to run desktop-file-validate on the desktop file, either in %install or %check. +desktop-file-validate %{buildroot}%{_datadir}/applications/btop.desktop https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_file_install_usage ================================================================================ The file %{_datadir}/btop/README.md is a duplicate of what we put in %{_docdir} (via %doc). I'd recommend rm-ing that file after the %make_install. ================================================================================ This current %files structure will result in %{_datadir}/btop being an unowned directory, which isn't allowed. You can remove the explicit listing of the contents of that directory and own it recursively. -%{_datadir}/btop/themes/HotPurpleTrafficLight.theme ...(and the rest)... +%{_datadir}/btop Same thing with %{_datadir}/icons/hicolor/, which we should resolve by requiring the package that does own it. +Requires: hicolor-icon-theme https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories/ ================================================================================ In case upstream adds more resolutions of their icon, it would be better to use a wildcard to pick them up. You could also glob the extension to avoid a separate line for the svg. -%{_datadir}/icons/hicolor/48x48/apps/btop.png -%{_datadir}/icons/hicolor/scalable/apps/btop.svg +%{_datadir}/icons/hicolor/*/apps/btop.* ================================================================================ That's what I noticed manually reviewing the spec file. I wasn't able to run fedora-review because it seems jonathanwright.me has certificate problems. Can you fix the cert or post them somewhere else with a working cert? -- 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=2107381 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
