https://bugzilla.redhat.com/show_bug.cgi?id=2246561 --- Comment #18 from blinxen <h-k-81@xxxxxxxxxxx> --- > PS: I might ask somebody else for a second opinion later since this is a rather complicated package with many components, and I'm not sure I'm getting 100% of the things right here. Sure, no problem :D > It's OK in the short-term, but please switch back to upstream sources as soon as the projects merge your PRs. I am already doing that :D I am also creating PRs [1] for upstream to use the newest version that includes the license commits. > 1. Looks like there's a file "/usr/share/licenses/helix/LICENSE-tree-sitter-." getting installed, this looks like a mistake? Should be fixed now. > 2. It seems that the grammar plugins that are getting compiled with "cc" link libm and libgcc_s even if they don't have to: Will report this upstream. > 3. There appear to be some files that have executable permissions that should not have them: Should be fixed now > 4. These shell completion scripts have shebangs but don't need them (I think)? I can report this upstream but it seems that this is not required but does not really do any damage if kept. > 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). Should be fixed now > 4. You install a .desktop file for Helix, but it is not validated, which is a mandatory requirement. Desktop file validation has been added > 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: Makes sense https://github.com/helix-editor/helix/pull/8691 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component 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%23c18 _______________________________________________ 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
