Bug 2119987

Summary:	Review Request: python-icapclient3 - Python3 module for creating ICAP clients
Product:	[Fedora] Fedora	Reporter:	Simone Caronni <negativo17>
Component:	Package Review	Assignee:	Frank Crawford <frank>
Status:	ASSIGNED ---	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	frank, mairacanal, package-review
Target Milestone:	---	Flags:	frank: fedora-review? frank: needinfo? (negativo17)
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:		Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Simone Caronni 2022-08-20 15:55:17 UTC

Spec URL: https://slaanesh.fedorapeople.org/python-icapclient3.spec
SRPM URL: https://slaanesh.fedorapeople.org/python-icapclient3-1.2.1-1.fc36.src.rpm
Description:
A Python3 module for creating ICAP clients. The module API is somewhat inspired
by the httplib python module.

This module is written in pure C, and uses the C-ICAP library to handle the ICAP
protocol.
Fedora Account System Username: slaanesh

Comment 1 Simone Caronni 2022-08-20 15:55:50 UTC

Depends on: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2119983

Comment 2 Maíra Canal 2022-08-24 17:39:49 UTC

Hi Simone,

Just a few nitpicks on your spec.

> %{?!python3_pkgversion:%global python3_pkgversion 3}

Using the %{python3_pkgversion} macro instead of using 3 on the BuildRequires packages, makes it harder to read. Usually, it is a better practice to drop the %{python3_pkgversion} macro.

> %global srcname icapclient3

Another readability nit, setting and using this makes the spec file harder to read. Using the literal name makes it easier to read. Moreover, sometimes you don't even use this macro and write "icapclient3" literally.

> %{?python_provide:%python_provide python3-%{srcname}}

This is deprecated and shouldn't be used. Check the note at the end of https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/#_the_py_provides_macro

NOTE: I'm not a packager yet and currently looking for sponsorship.

Best Regards,
- Maíra Canal

Comment 3 Frank Crawford 2023-08-13 08:36:41 UTC

@negativo17 I've done a review, and everything pretty much passes.  There are just three items I want to confirm before publishing it:

> [ ]: Provides: bundled(gnulib) in place as required.
>      Note: Sources not installed
I don't believe there is any bundled(gnulib), and this is just a bug in fedora-review.
Is that correct?

> [ ]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include 
I know you have included the LICENSE file from upstream, and also that there has been no new release for some years, but have you or can you add an issue upstream for it to be included in any future release.

> [ ]: %check is present and all tests pass.
I assume there is no suitable checks and it is unlikely ever to occur.
Am I correct about that?

Once I get confirmation on those points, I'll publish the review and we should be good to go.

Comment 4 Frank Crawford 2023-08-13 08:38:48 UTC

P.S. I assume the response to Maíra Canal is because you have set up the spec file to be compatible with EPEL, including even EPEL7?