https://bugzilla.redhat.com/show_bug.cgi?id=2414840



--- Comment #13 from Ben Beasley <[email protected]> ---
It’s a minor thing, but this doesn’t do what you intended:

%if 0%{?el} <= 9

On Fedora, the el macro is not defined, and this expands to "%if 00 <= 9",
which is true – so the unnecessary manual .la file removal does happen on
Fedora.

Since the only non-end-of-life EPEL older than EPEL9 is EPEL8, you might just
write "%if 0%{?el8}" or "%if %{defined el8}" instead.

Explicitly removing the .la file is just unnecessary on Fedora (because it’s
handled by a buildroot policy script), not wrong or harmful, so I wouldn’t
block a package review on this.

----

You should be more explicit about listing files in shared directories,

%{_includedir}/*.h
%{_includedir}/*.F90
%{_libdir}/*.so
[…]
%{_libdir}/pkgconfig/*.pc

See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists.

----

A more straightforward way to write this

  Source0:
%{url}/archive/refs/tags/v%{version}.tar.gz#/%{name}-%{version}.tar.gz

would be this

  Source0: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz

If you want to be very precise about specifying a tag (only really required if
upstream
produces releases with the same names as tags, making the URL I’ve suggested
above
ambiguous), then you can also write

  Source0: %{url}/archive/refs/tags/v%{version}/%{name}-%{version}.tar.gz

which still avoids the "#/" hack.

----

I favor re-generating configure scripts from autotools sources by adding
BuildRequires on autoconf, automake, and libtool, and adding "autoreconf
--force --install --verbose" to %build (or to %conf, if using that section),
rather than using pre-generated configure scripts, but this has been a matter
of significant debate in Fedora, and there has never been a solid consensus on
what the Right Thing to Do is. Therefore, it wouldn’t be correct to ask you to
change this, only to point out that you have a choice.

----

Is there anything stopping you from running the tests?

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites

----

I still haven’t looked at this as closely as I would when doing a real review,
but this otherwise looks pretty reasonable.


-- 
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=2414840

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202414840%23c13

-- 
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue

Reply via email to