https://bugzilla.redhat.com/show_bug.cgi?id=2010116
Carl George 🤠 <carl(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |carl(a)redhat.com
Doc Type|--- |If docs needed, set a value
Assignee|nobody(a)fedoraproject.org |carl(a)redhat.com
Flags| |fedora-review?
--- Comment #3 from Carl George 🤠 <carl(a)redhat.com> ---
I believe this section should reference the macros where applicable.
(ie %{__mkdir_p}, %{__cp}, %{__install}). This is a stylistic choice.
Macro forms of system executables SHOULD NOT be used except when there is a
need to allow the location of those executables to be configurable. mkdir, cp,
and install are the right choice here.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros
The spec should directly reference the names of these files.
Agreed. The Python guidelines state this explicitly, but it's also a good
general guideline. I'm probably going to work on getting a generic form into
the main guidelines.
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_explic...
Here are a few additional things I noticed that should be fixed.
The metainfo file should be installed into %{_metainfodir}.
-%{appdata_dir}/vim-surround.metainfo.xml
+%{_metainfodir}/vim-surround.metainfo.xml
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
It's not directly mentioned in the guidelines, but similar to the patch status
guideline, there should be a comment for Source1 linking to where the metainfo
file was submitted upstream.
https://docs.fedoraproject.org/en-US/packaging-guidelines/PatchUpstreamSt...
The %post and %postun scriptlets are no longer necessary on Fedora or EPEL8+
due to file triggers in the main vim package. If and only if you plan to add
this package to EPEL7, you can keep them with a conditional so they only apply
on EPEL7. I suggest following the example of the docfiletriggers conditional
in the vim-fugitive spec file. If you remove them, make sure to also remove
the corresponding Requires(post) and Requires(postun).
https://src.fedoraproject.org/rpms/vim/c/d76e3c95ac644b1ab577bf4db79fb055...
https://src.fedoraproject.org/rpms/vim-fugitive/blob/rawhide/f/vim-fugiti...
Since the review was submitted, upstream has released version 2.2, so the spec
file should be updated to that.
https://github.com/tpope/vim-surround/releases/tag/v2.2
--
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=2010116