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



--- Comment #14 from Phil Wyett <[email protected]> ---
(In reply to Ben Beasley from comment #13)
> 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.

I think my new upload should cover the issues raised.

Spec URL:
https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/giza.spec

SRPM URL:
https://kathenas.fedorapeople.org/development/fedora/rawhide/for_review/giza-1.5.0-1.el9.src.rpm


-- 
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%23c14

-- 
_______________________________________________
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