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



--- Comment #8 from Miro Hrončok <[email protected]> ---
(In reply to Miro Hrončok from comment #5)
> Please use raw links to spec+srpm, not to HTML pages with them. That allows
> automatic checks via Fedora Review. Thanks.

spec file is still linked to a HTML page.

> > %global pyname mass_prebuild
> 
> This macro is only used once. Consider replacing that usage with
> mass_prebuild verbatim.


Fixed.


> > Release:        3%{?dist}
> 
> %autorelese is the preferred way in Fedora.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_release_tag

Fixed.

> > Summary:        A set of tools to massively pre-build reverse dependencies 
> > for a package
> 
> Consider saying this is for RPM packages.

Fixed. I think it's "an RPM package", not "a RPM package", but I am not a
native speaker.


> > Patch:          drop-setuptools_scm.patch
> 
> This patch only includes an explanation within the patch itself. However,
> each patch reason should be explained in the specfile.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines
> 
> 
> > Patch:          0001-docs-license-Update-GPL-to-latest-version.patch
> 
> Same here + please don't change licenses downstream.


Both patches removed.


> > # The pyproject.toml file needs to be updated upstream to had more of these.
> > BuildRequires:  python3-copr
> > ...
> 
> Consider explaining what those are. Are those upstream-undeclared runtime
> dependencies? Or test dependencies? Or actual build dependencies?

This was improved.

BuildRequires:  python3-pip  <- this one is redundant and generated by
%pyproject_buildrequires (unless you need it for other reasons).

> > install -d %{buildroot}%{_sysconfdir}/bash_completion.d
> 
> Please use %{bash_completions_dir}.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/ShellCompletions/
> 
> > %config(noreplace) %{_sysconfdir}/bash_completion.d/mpb*
> 
> If, for some reason, you actually need the bash completions in /etc, do you
> expect users to modify them themselves? The %config(noreplace) notation
> seems to be abused here.

Both fixed.


> > %license LICENSE
> 
> You use `%pyproject_save_files -l` hence manually listing a %license file
> should not be necessary.

Fixed.

----


Two notable problems caught by fedora-review-service:



     No known owner of /etc/mpb/repo.conf.d, /etc/mpb/copr.conf.d

This is correct, the %files should list %dir %{_sysconfdir}/mpb/*.conf.d as
well, not only the content.




    [!]: Uses parallel make %{?_smp_mflags} macro.

The spec file uses plan `make` where it likely should invoke %make_build -- see
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make


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

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

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