https://bugzilla.redhat.com/show_bug.cgi?id=2106138 --- Comment #8 from Felix Kaechele <felix@xxxxxxxxxxx> --- I'm a bit confused as to who is submitting this package for review at this point? Or are neil and supakeen the same person? Other than that, here are a few more comments, going through the spec file top to bottom: 1. For the "Release" field use %autorelease: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_release_tag 2. Did you look at https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_tag_example (as part of https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_using_forges_hosted_revision_control) that I linked earlier? If you find that too complicated you can instead also cheat using anchor links to get a properly named source file: Instead of Source: %{url}/archive/refs/tags/v%{version}.tar.gz use Source: %{url}/archive/refs/tags/v%{version}.tar.gz#/%{name}-%{version}.tar.gz That way you can also drop the "-n" parameter on the `%autosetup` macro. 3. It's up to you whether you want to carry the changes to the systemd unit file as a patch. I find it more work possibly having to rework the patch if it no longer cleanly applies. Simply carrying the entire systemd unit file as an extra `Source` or using `sed` in the `%prep` section (see an example here: https://src.fedoraproject.org/rpms/borgmatic/blob/3f772e679a94c823337ff22084166e3346cb4020/f/borgmatic.spec#_48) would be acceptable as well. 4. What's the purpose of defining the description via %global _description and then doing %description %_description? %_description is used nowhere else so it may be redundant. 5. The systemd unit is being installed into the wrong location. It needs to go in `%{buildroot}%{_unitdir}/%{name}.service`. Also, using `install -Dpm` instead of `cp -a` allows you to drop the `mkdir` lines. See for example: https://src.fedoraproject.org/rpms/borgmatic/blob/3f772e679a94c823337ff22084166e3346cb4020/f/borgmatic.spec#_49 Other than that it looks pretty good to me. Some other choices you made (like how you sorted the %files section) come down to personal preference/style and they won't be part of the formal review. -- 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=2106138 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202106138%23c8 _______________________________________________ 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
