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



--- Comment #2 from Otto Liljalaakso <[email protected]> ---
Thank you for reviewing!

(In reply to fedepell from comment #1)
> First minor/major comment: saw that just today version 0.10.0 was released
> including your patch (that is in the SRPM): maybe worth bumping directly to
> that?

Nice, updated to the latest version.

> > Source:         %{url}/archive/v%{version}/v%{version}.tar.gz
> > # Remove shebang from a file that is not intended to be executed
> > Patch:          https://github.com/xattr/xattr/pull/106.patch
> 
> Can use %{url} also for patch?

Makes sense, but not applicable for 0.10.0 which does not need the patch.

> > BuildRequires:  gcc
> > BuildRequires:  pytest
> > BuildRequires:  python3-devel
> 
> Usually I see it's norm to put a comment before the testing ones (just
> pytest in this case)

I suppose it is a matter of style,
however I moved the test dependency to the end of the list and added such
comment.

> > %doc CHANGES.txt
> > %doc README.rst
> 
> Could put all in a line? Maybe add as well TODO.txt

Matter of style again, however I applied you suggestion.
I also added TODO.txt.

> Maybe add also "%license LICENSE.txt" ?

Uh, this was a bad mistake. Added.

Spec URL: http://oturpe.kapsi.fi/fedora/rpms/python-xattr/2/python-xattr.spec
SRPM URL:
http://oturpe.kapsi.fi/fedora/rpms/python-xattr/2/python-xattr-0.10.0-1.fc38.src.rpm


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