Hello,

 - Please use a more meaningful name for the archive, with:

Source0:	https://github.com/defeo/%{modname}/archive/%{version}/%{modname}-%{version}.tar.gz

 - Please package the latest version, currently 1.1.3: https://github.com/defeo/cypari2/releases/tag/1.1.3 Note that README.md has been replaced by README.rst, don't forget to update this in %files.

Package is fine otherwise, just fix the aforementioned points and it will be accepted.

Hi, many thanks for the initial review.

At first I would prefer keep version 1.0.0 because it is the version
used by sagemath 8.0. But after testing, later version should work with
sagemath 8.0.

I changed the Source line, but it will cause fedora-review to not be able
to download and validate it, other automated tools may also have trouble
with it.

Later version has documentation and self tests. Now added doc subpackage
and %check section.

Update:

Spec URL: https://pcpa.fedorapeople.org/python-cypari2.spec
SRPM URL: https://pcpa.fedorapeople.org/python-cypari2-1.1.3-1.fc28.src.rpm

>but it will cause fedora-review to not be able to download and validate it

No, it works fine. That is if you actually put the correct URL, which you didn't do:

Source0:	https://github.com/defeo/%{modname}/archive/%{version}/%{modname}-%{version}.tar.gz


The tests you've added don't work for me:

  File "tests/rundoctest.py", line 11, in <module>
    import cypari2
  File "/builddir/build/BUILDROOT/python-cypari2-1.1.3-1.fc28.x86_64/usr/lib64/python2.7/site-packages/cypari2/__init__.py", line 1, in <module>
    from .pari_instance import Pari
ImportError: /builddir/build/BUILDROOT/python-cypari2-1.1.3-1.fc28.x86_64/usr/lib64/python2.7/site-packages/cypari2/pari_instance.so: undefined symbol: sig_off
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.mUMhJO (%check)
    Bad exit status from /var/tmp/rpm-tmp.mUMhJO (%check)
Child return code was: 1

  Ops,

  Thanks for noting my error in the URL (s/name/modname/).

  I also forgot that to generate docs and run check, need to update
to a newer python-cysignals.

  If you want to check it I can upload the python-cysignals srpm.

  I am delaying it for a batch of required updates, before building
sagemath 8.0.

Update:

- Correct Source URL
- Temporarily disable doc and check as need to update cysignals

Spec URL: https://pcpa.fedorapeople.org/python-cypari2.spec
SRPM URL: https://pcpa.fedorapeople.org/python-cypari2-1.1.3-2.fc28.src.rpm

> If you want to check it I can upload the python-cysignals srpm.

Can't you just build it on Koji? Or link me to a scratch build of it so I can snatch the RPMs.

I prefer to not build a final package in koji as it may take significant
time to get all bits in place to build a newer sagemath, so, I prefer
to avoid an inconsistent state.

Scratch build of newer python-cysignals at:

https://koji.fedoraproject.org/koji/taskinfo?taskID=23026237

Updated package to build requires correct version, generate
doc subpackage and run %check:

Spec URL: https://pcpa.fedorapeople.org/python-cypari2.spec
SRPM URL: https://pcpa.fedorapeople.org/python-cypari2-1.1.3-3.fc28.src.rpm

Test fails:

File "/builddir/build/BUILDROOT/python-cypari2-1.1.3-3.fc28.x86_64/usr/lib64/python3.6/site-packages/cypari2/gen.cpython-36m-x86_64-linux-gnu.so", line ?, in cypari2.gen.__test__.list_of_Gens_to_Gen (line 4861)
Failed example:
    from six.moves import range
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib64/python3.6/doctest.py", line 1330, in __run
        compileflags, 1), test.globs)
      File "<doctest cypari2.gen.__test__.list_of_Gens_to_Gen (line 4861)[0]>", line 1, in <module>
        from six.moves import range
    ModuleNotFoundError: No module named 'six'

Guess you need to add a BR to python-six.

I'll finish this tomorrow if you don't mind.

Fully verified, and indeed only python{2,3}-six BR were required.

Spec URL: https://pcpa.fedorapeople.org/python-cypari2.spec
SRPM URL: https://pcpa.fedorapeople.org/python-cypari2-1.1.3-4.fc28.src.rpm

All ok, package accepted.

Many thanks for the review!

(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-cypari2. You may commit to the branch "f27" in about 10 minutes.

There was an a crash on 32 bit builds during %check, but the problem
would happen on 64 bit as well, just not crash.
Reported, with patch, at https://github.com/defeo/cypari2/issues/46

Pacgake already present in 28 so review can be probably closed.