Copr build:
https://copr.fedorainfracloud.org/coprs/build/10757483
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2502852-python-magnumclient/fedora-rawhide-x86_64/10757483-python-magnumclient/fedora-review/review.txt

Found issues:

- License file AUTHORS is not marked as %license
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text
- A package with this name already exists. Please check https://src.fedoraproject.org/rpms/python-magnumclient
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

I was just about to submit my own version of this

https://straylen.web.cern.ch/rpms/python-magnumclient/

I will of course review this one since you beat me to to it by a week :-)

> %{!?sources_gpg: %{!?dlrn:%global sources_gpg 1} }

What is "dlrn" - when would this be set ?

> flake8-import-order  

has been excluded as a BR but it was never present in the first place ?

> BuildRequires:  pyproject-rpm-macros

This is not needed - it is already part of the koji buildroot for fedora  (and EPEL )

> sed -i /^minversion.*/d tox.ini

This is redundant really , there is no need to remove that constraint and it us useful moving forward.

> %if ! 0%{?with_doc}
> %global excluded_brs %{excluded_brs} sphinx openstackdocstheme
> %endif 

This is not needed , there is already a with_doc condition on the %tox macro and those deps are not included when `-e %{default_toxenv}` is used with docs.

Why version 4.7 and not latest ?

(In reply to Steve Traylen from comment #3)
> > %{!?sources_gpg: %{!?dlrn:%global sources_gpg 1} }
> 
> What is "dlrn" - when would this be set ?
> 
> > flake8-import-order  
> 
> has been excluded as a BR but it was never present in the first place ?
> 
> > BuildRequires:  pyproject-rpm-macros
> 
> This is not needed - it is already part of the koji buildroot for fedora 
> (and EPEL )
> 
> > sed -i /^minversion.*/d tox.ini
> 
> This is redundant really , there is no need to remove that constraint and it
> us useful moving forward.
> 
> > %if ! 0%{?with_doc}
> > %global excluded_brs %{excluded_brs} sphinx openstackdocstheme
> > %endif 
> 
> This is not needed , there is already a with_doc condition on the %tox macro
> and those deps are not included when `-e %{default_toxenv}` is used with
> docs.

Hi Steve, thank you for reviewing my package review request and the valuable feedback.

I've took the existing rpm spec of when it was still maintained and present in Fedora (https://src.fedoraproject.org/rpms/python-magnumclient/blob/18958c3b35f477c23468f4fcf6ed6006ddd6a378/f/python-magnumclient.spec), cleaned it up and updated it. It seems there are some unneeded leftovers. I'll gladly clean those up as well.

(In reply to Steve Traylen from comment #4)
> Why version 4.7 and not latest ?

Regarding the version: As I am currently using the copr repository to install the package on my client, I've built all versions up to the latest version 4.11.0 (4.7.1 just happens to be the first version that wasn't built and present in the fedora repository after the package had been orphaned).

The latest spec file as well as the latest srpm may be found here:
- https://download.copr.fedorainfracloud.org/results/g3ntleli0n/python-magnumclient/srpm-builds/10757698/python-magnumclient.spec
- https://download.copr.fedorainfracloud.org/results/g3ntleli0n/python-magnumclient/srpm-builds/10757698/python-magnumclient-4.11.0-1.src.rpm

Created attachment 2153377 [details]
The .spec file difference from Copr build 10757483 to 10806659

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10806659
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2502852-python-magnumclient/fedora-rawhide-x86_64/10806659-python-magnumclient/fedora-review/review.txt

Found issues:

- License file AUTHORS is not marked as %license
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text
- A package with this name already exists. Please check https://src.fedoraproject.org/rpms/python-magnumclient
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Hi Luca,

> I've took the existing rpm spec of when it was still maintained and present in Fedora 

This is a re-review so we are really starting again. Its as if the  package was new ? We can't just bring it back as it was.

Other points still stand:

* dlrn variables are never set in Fedora as far as I know so does not belong a a fedora spec file.
* flake8 is not a dependency anyway so why is it being removed?
* The minversion in tox.ini file is not undesirable - why is it being removed ?
* sphinx is not installed anyway when the doc tox env is used.

Most if not all the other openstack packages and now been adapted away from RDO which no longer exists, when this package was in F43 RDO still existed.

Steve.

p.s I be greatful if openstack-sig could be added as a maintainer - I am happy to maintin in that context with the others I am doing: https://fedora-openstack.web.cern.ch/

(In reply to Steve Traylen from comment #9)
> Hi Luca,
> 
> > I've took the existing rpm spec of when it was still maintained and present in Fedora 
> 
> This is a re-review so we are really starting again. Its as if the  package
> was new ? We can't just bring it back as it was.
> 
> Other points still stand:
> 
> * dlrn variables are never set in Fedora as far as I know so does not belong
> a a fedora spec file.
> * flake8 is not a dependency anyway so why is it being removed?
> * The minversion in tox.ini file is not undesirable - why is it being
> removed ?
> * sphinx is not installed anyway when the doc tox env is used.
> 
> Most if not all the other openstack packages and now been adapted away from
> RDO which no longer exists, when this package was in F43 RDO still existed.
> 
> Steve.

Hi Steve,

Thanks for your feedback. I have cleaned up the .spec file using your inputs and I have just built all the packages using the latest commit. The updated spec file may be found here:
- https://download.copr.fedorainfracloud.org/results/g3ntleli0n/python-magnumclient/fedora-44-x86_64/10807621-python-magnumclient/python-magnumclient.spec

(In reply to Steve Traylen from comment #10)
> p.s I be greatful if openstack-sig could be added as a maintainer - I am
> happy to maintin in that context with the others I am doing:
> https://fedora-openstack.web.cern.ch/

sure thing :)

Okay.

If happy to give co-maintainership to openstack-sig like it had before then I can approve.

I may make some changes after but I will do that always via merge request so you will have opportunity to comment.

Steve.

Approved.

This is unapproved.

I had not realised you were not yet a packager.

I am happy to sponsor you but I cannot based on this package alone.

To be a sponsoring review I have to be a lot stricter.

Please check all the build deps you are excluding , not just the flake8 ones there are others in that list.


The %check section does `|| true` this makes no sense or at least should be documented as to why.

*** This bug has been marked as a duplicate of bug 2511009 ***