https://bugzilla.redhat.com/show_bug.cgi?id=2246561 --- Comment #17 from Fabio Valentini <decathorpe@xxxxxxxxx> --- A few more minor issues with the spec file: 1. Inconsistent indentation: The tags from Source100 up to the Provides for bundled tree-sitter grammars are indented inconsistently from the rest of the spec file, which uses 16 columns of indentation. For the bundled Provides, you can just tweak the command that you used to generate them (i.e. add the appropriate amount of spaces in the command at the right position). 2. The list of bundled Provides for tree-sitter grammars is unsorted. I suspect that if you regenerate this list with the next version, it will have a different order. I suggest to add something like "| sort" at the end of command that you used to generate the list. 3. The "%{_libdir}/helix/" directory ends up unowned. The package only owns "%{runtime_directory_path}", which is a subdirectory of "%{_libdir}/helix/", but it doesn't own the parent directory. Please add something like "%dir %{_libdir}/helix" to the %files list as well. 4. You install a .desktop file for Helix, but it is not validated, which is a mandatory requirement. You will need to add "BuildRequires: desktop-file-utils" (which is already there, because you use desktop-file-install), and run something like this in %check (possibly unconditionally, whether %bcond check is enabled or not), similar to this: """ %check desktop-file-validate %{buildroot}/%{_datadir}/applications/Helix.desktop %if %{with check} # Grammars are already built export HELIX_DISABLE_AUTO_GRAMMAR_BUILD=true %cargo_test %{buildroot}%{_bindir}/%{binary_name} --health %endif """ 5. The other steps for preparing license files for installation all happen in %prep, but this one happens in %install, even though it does not *install* anything, just moves things around in the build directory: > # Rename license files for tree-sitter grammars so they can be installed I think it would make sense to move that to %prep as well. -- 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=2246561 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202246561%23c17 _______________________________________________ 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, report it: https://pagure.io/fedora-infrastructure/new_issue
