> Spec URL: https://github.com/luigifab/python-radexreader/blob/master/fedora/python3-radexreader.spec

When linking a SPEC file, please point the URL to the raw content.
Also, the SPEC file should be called "python-radexreader.spec".

> Name:        python3-radexreader

Same here, the name should be "python-radexreader"

> URL:         https://github.com/luigifab/python-radexreader
> Source0:     https://github.com/luigifab/python-radexreader/releases/download/v%{version}/%{name}-%{version}.tar.gz

You can re-use URL in the Source0 field as %{url} like so:
%{url}/releases/download/v%{version}/%{name}-%{version}.tar.gz

Also, the Source0 URL is incorrect. It should be like this:
https://github.com/luigifab/python-radexreader/releases/download/v1.0.0/python-radexreader-1.0.0.tar.gz
Notice that the name is actually "python-radexreader".

> BuildRequires: python3-devel
> Requires: python3
> Requires: python3-pyusb

Python module Requires and BuildRequires should follow the format "python3dist(foo)".

The SPEC file is missing the python3-radexreader subpackage, which is defined like this:
%package -n     python3-radexreader
Summary:        %{summary}
%py_provides python3-radexreader

%description -n python3-radexreader
Description goes here

You can define %global variables for "radexreader" and the %description blocks to avoid duplication, like so:
%global pypi_name radexreader

%global common_description %{expand:
A multiple line description goes here.}

> %install
> %py3_install
> mkdir -p %{buildroot}/lib/udev/rules.d/ %{buildroot}/usr/bin/
> cp -a udev.rules      %{buildroot}/lib/udev/rules.d/60-python3-radexreader.rules
> cp -a bin/radexreader %{buildroot}/usr/bin/radexreader
> chmod +x %{buildroot}/usr/bin/radexreader

The last 2 lines can probably be replaced with an "install" call:
install -p -m755 bin/radexreader %{buildroot}/usr/bin/radexreader

Also, you should avoid using "/usr/bin" directly and use the "%{_bindir}" macro instead.

> %files
> %{python3_sitelib}/radexreader/
> %{python3_sitelib}/radexreader*egg-info/
> /lib/udev/rules.d/60-python3-radexreader.rules
> /usr/bin/radexreader

The files should belong to the python3-radexreader subpackage.

I'll run the full review matrix once these are fixed.

Hum, ok, so my spec is very bad...
Here is a new ones.

Spec URL: https://gist.githubusercontent.com/luigifab/0eb7ec0b7088ae6ced097ce31b3cacf7/raw/362b9087d55affb2f81045290db81f81f04e5d6c/python-radexreader.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/2378/55642378/python-radexreader-1.0.0-1.fc32.src.rpm
KOJI BUILD: https://koji.fedoraproject.org/koji/taskinfo?taskID=55642377

I'm not sure, I used:
> BuildRequires: python3-devel
> Requires: python3
> Requires: %{py3_dist pyusb}

I will update the spec of the main repo later.
I also updated my specs for my other packages.


But for this package and my other packages, there is something that I don't understand.
Some file (here the udev file), are not in the Source0 archive.
Does I add a "Source1"?

For example: "Source1: https://raw.githubusercontent.com/luigifab/python-radexreader/master/debian/udev"

Thanks for your review.

I read more docs, and yes, Source1 is a good idea.

I updated the main repository.
Spec URL: https://github.com/luigifab/python-radexreader/raw/master/fedora/python-radexreader.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/4636/55674636/python-radexreader-1.0.0-1.fc32.src.rpm
KOJI BUILD: https://koji.fedoraproject.org/koji/taskinfo?taskID=55674635

I hope it is better.
Sorry for my multiple attempts.

The SPEC file looks a lot better now! Good job! :)

> BuildRequires: python3-devel
> Requires: python3
> Requires: %{py3_dist pyusb}

I think the last line should be:
> Requires: python3dist(pyusb)

Have a look at the related section in the Python Packaging Guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_dependencies

> %files -n python3-radexreader
> %{python3_sitelib}/radexreader/
> %{python3_sitelib}/radexreader*egg-info/
> /lib/udev/rules.d/60-python3-radexreader.rules
> %{_bindir}/radexreader

You should also include the LICENSE file with the %license tag and the README.md file with the %doc tag.

From fedora-review:
- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in
  /data/rpmbuild/SPECS/python-radexreader/python-radexreader/diff.txt
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/

I think there's been a mismatch between the upstream tarball and the sources used in the RPM and SRPM.

From rpmlint:
> Rpmlint
> -------
> Checking: python3-radexreader-1.0.0-1.fc32.noarch.rpm
>           python-radexreader-1.0.0-1.fc32.src.rpm
> python3-radexreader.noarch: I: enchant-dictionary-not-found fr

You need to add a BuildRequires on aspell-fr or hunspell-fr for the extra language checks to work properly.

> python3-radexreader.noarch: W: no-documentation
> python3-radexreader.noarch: E: non-executable-script /usr/lib/python3.8/site-packages/radexreader/__init__.py 644 /usr/bin/python3

You should remove the "/usr/bin/python3" shebang from that file, for instance using "sed".

Full review below:
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/
- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in
  /data/rpmbuild/SPECS/python-radexreader/python-radexreader/diff.txt
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/


===== MUST items =====

Generic:
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
     Note: Using prebuilt packages
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[!]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GNU Lesser General Public License", "GNU General Public
     License". Detailed output of licensecheck in
     /data/rpmbuild/SPECS/python-radexreader/python-
     radexreader/licensecheck.txt
[x]: Package requires other packages for directories it uses.
     Note: No known owner of /lib/udev, /lib/udev/rules.d
     Review: bogus
[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/python3.8,
     /usr/lib/python3.8/site-packages, /lib/udev/rules.d, /lib/udev
     Review: bogus as well
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[!]: Rpmlint is run on all installed packages.
     Note: Mock build failed
     See: https://docs.fedoraproject.org/en-US/packaging-
     guidelines/#_use_rpmlint
[!]: Spec file according to URL is the same as in SRPM.
     Note: Bad spec filename: /data/rpmbuild/SPECS/python-
     radexreader/python-radexreader/srpm-unpacked/python-radexreader.spec
     See: (this test has no URL)


Installation errors
-------------------
INFO: mock.py version 2.6 starting (python version = 3.8.6)...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
INFO: Signal handler active
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled package manager cache
Start: cleaning package manager metadata
Finish: cleaning package manager metadata
INFO: enabled HW Info plugin
Mock Version: 2.6
INFO: Mock Version: 2.6
Finish: chroot init
INFO: installing package(s): /data/rpmbuild/SPECS/python-radexreader/python3-radexreader-1.0.0-1.fc32.noarch.rpm
ERROR: Command failed: 
 # /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 34 --setopt=deltarpm=False --allowerasing --disableplugin=local --disableplugin=spacewalk install /data/rpmbuild/SPECS/python-radexreader/python3-radexreader-1.0.0-1.fc32.noarch.rpm --setopt=tsflags=nocontexts



Rpmlint
-------
Checking: python3-radexreader-1.0.0-1.fc32.noarch.rpm
          python-radexreader-1.0.0-1.fc32.src.rpm
python3-radexreader.noarch: I: enchant-dictionary-not-found fr
python3-radexreader.noarch: W: no-documentation
python3-radexreader.noarch: E: non-executable-script /usr/lib/python3.8/site-packages/radexreader/__init__.py 644 /usr/bin/python3 
python3-radexreader.noarch: W: no-manual-page-for-binary radexreader
2 packages and 0 specfiles checked; 1 errors, 2 warnings.




Source checksums
----------------
https://github.com/luigifab/python-radexreader/raw/v1.0.0/debian/udev :
  CHECKSUM(SHA256) this package     : fb5279baa7aa4962667705fbb42dbfd0ea45ef574a3f53c02a6ca14fc3e9f150
  CHECKSUM(SHA256) upstream package : fb5279baa7aa4962667705fbb42dbfd0ea45ef574a3f53c02a6ca14fc3e9f150
https://github.com/luigifab/python-radexreader/releases/download/v1.0.0/python-radexreader-1.0.0.tar.gz :
  CHECKSUM(SHA256) this package     : 6d55312383b536777eb0db35864c96669e1ae6749487d6e5e9ba4a3c521e3679
  CHECKSUM(SHA256) upstream package : f5b9922fbceb9dae5946649caac5a076e72e4364d9cc49b62452d7a959294cfd
diff -r also reports differences


Requires
--------
python3-radexreader (rpmlib, GLIBC filtered):
    /usr/bin/python3
    python(abi)
    python3.8dist(pyusb)



Provides
--------
python3-radexreader:
    python-radexreader
    python3-radexreader
    python3.8dist(radexreader)
    python3dist(radexreader)

You are so right! I fixed. I rebuilded. I updated.

Spec URL: https://github.com/luigifab/python-radexreader/raw/master/fedora/python-radexreader.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/772/55750772/python-radexreader-1.0.0-1.fc32.src.rpm
KOJI BUILD: https://koji.fedoraproject.org/koji/taskinfo?taskID=55750759

I haven't found an "OR" directive for BuildRequires, so I chose hunspell-fr.
For the README.md, I tried to add an URL at the begining of the file.

> %prep
> ...
> sed -i '1i %{url}\n' %{SOURCE2}

Result, something is wrong: W: file-size-mismatch README.md = 3902, https://.../README.md = 3860
I think this is because I use %{SOURCE2} instead of README.md in %prep section.
But if I use README.md, file isn't found...

(I will update my other packages soon)

Okay, I found a way.

Spec URL: https://github.com/luigifab/python-radexreader/raw/master/fedora/python-radexreader.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/4743/55754743/python-radexreader-1.0.0-1.fc32.src.rpm
KOJI BUILD: https://koji.fedoraproject.org/koji/taskinfo?taskID=55754742

I'm not sure, but not error here (in the previous build I didn't see the cp: cannot stat 'README.md': No such file or directory).
> %prep
> ...
> cp %{SOURCE2} README.md
> sed -i '1i %{url}\n' README.md

> Source1:     %{url}/raw/v%{version}/debian/udev
> Source2:     %{url}/raw/v%{version}/README.md

I had a look at the source tarball again and you don't actually have to attach additional sources with SourceX tags, since everything is already in the tarball. Please, remove these lines.

> %prep
> %setup -q
> sed -i 's/python-radexreader /python-radexreader-rpm /g' cmd.py
> sed -i 's/#!\/usr\/bin\/python3/#/g' radexreader/__init__.py

I might be wrong, but I think the shebang and comment hashes need to be escaped in the "sed" call like so:
> sed -i 's/\#\!\/usr\/bin\/python3/\#/g' radexreader/__init__.py

> sed -i '1i %{url}\n' README.md

Since you're the upstream developer of python-radexreader, I think it's better to include the URL in the source tree, rather than modifying it like this in the SPEC file. Also, that's why you got a checksum mismatch on the README.md file.

I cleaned up the SPEC file a little:
> %prep
> %setup -q
> sed -i 's/python-radexreader /python-radexreader-rpm /g' cmd.py
> sed -i 's/\#\!\/usr\/bin\/python3/\#/g' radexreader/__init__.py

Added escapes to the second "sed" call, removed SOURCE2.

> %install
> %py3_install
> mkdir -p %{buildroot}/lib/udev/rules.d/ 
> mkdir -p %{buildroot}%{_datadir}/doc/python3-radexreader/ 
> mkdir -p %{buildroot}%{_bindir}/
> install -p -m 644 debian/udev %{buildroot}/lib/udev/rules.d/60-python3-radexreader.rules
> install -p -m 755 cmd.py %{buildroot}%{_bindir}/radexreader

Split the "mkdir" calls into multiple lines for clarity and later easier creating patches. udev rules are now installed from the "debian" dir in the source tarball, rather than being added via a SourceX tag.

Regarding sponsorship, please have a look at this doc: https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
And this doc: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join
The section about the FE-NEEDSPONSOR bug report tracker is especially important.

When you say the "source tarball", is it?
> URL:         https://github.com/luigifab/python-radexreader
> Source0:     %{url}/releases/download/v%{version}/%{name}-%{version}.tar.gz
= https://github.com/luigifab/python-radexreader/releases/download/v1.0.0/python-radexreader-1.0.0.tar.gz

I'm confused because this archive doesn't include Source1 and Source2.
I will update my spec and rebuild today.

> I'm confused because this archive doesn't include Source1 and Source2.

There are actually 2 archives:
- generated explicitly: https://github.com/luigifab/python-radexreader/releases/download/v1.0.0/python-radexreader-1.0.0.tar.gz
- GitHub tarball, generated automatically by GitHub: https://github.com/luigifab/python-radexreader/archive/v1.0.0/python-radexreader-1.0.0.tar.gz

I think it's preferable to use the latter, but yes, you're correct, the first one doesn't contain Source1 and Source2. Apologies.
Could you use the following for your Source0?
> %{url}/archive/v%{version}/%{name}-%{version}.tar.gz

That's the full source tarball.

Okay, why not.

Here is new build (I also updated my other packages in same way):
Spec URL: https://github.com/luigifab/python-radexreader/raw/master/fedora/python-radexreader.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/8937/55898937/python-radexreader-1.0.0-1.fc32.src.rpm
KOJI BUILD: https://koji.fedoraproject.org/koji/taskinfo?taskID=55898936

I kept the change in README.md to allow people that read the /usr/share/doc/python3-radexreader/README.md to known the "website" of the app.

> URL:         https://github.com/luigifab/python-radexreader
> Source0:     %{url}/archive/v%{version}.tar.gz

I see the Source0 field is still the same. One of the reasons "%{name}-%{version}.tar.gz" is arguably better, is because it clearly identifies your tarball. "v%{version}.tar.gz" is very ambiguous and can get easily mixed up with other packages and tarballs.

> I kept the change in README.md to allow people that read the /usr/share/doc/python3-radexreader/README.md to known the "website" of the app.

I still think it makes more sense to have an extra section in the original README (in the source tree, committed to git) with the URL, rather than modifying the sources in the SPEC file. You can then cut a patch release, 1.0.1, with the change and everyone who has access to the sources can refer to that feature.

Wow, sorry, I did not read correctly your suggestion for Source0, I changed. I also removed the change of the README in spec. I will prepare a section in the original README soon.

I rebuilded:
Spec URL: https://github.com/luigifab/python-radexreader/raw/master/fedora/python-radexreader.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/2415/56072415/python-radexreader-1.0.0-1.fc32.src.rpm
KOJI BUILD: https://koji.fedoraproject.org/koji/taskinfo?taskID=56072302

Apologies for the delay!

Looks good now, approved!

Hi, I updated to version 1.1.0 (https://github.com/luigifab/python-radexreader/raw/master/fedora/python-radexreader.spec).
Koji rebuild: https://koji.fedoraproject.org/koji/taskinfo?taskID=65107789 (SRPM https://kojipkgs.fedoraproject.org//work/tasks/7790/65107790/python-radexreader-1.1.0-1.fc35.src.rpm)

At the same time, I was sponsored for another packages (awf-gtk2 & awf-gtk3), can I request-repo for this package and remove Blocks:FE-NEEDSPONSOR?

Yesterday, I tried to build the RPM for OpenSuse with my SPEC, and I found two mistakes.

First, I think that:
> Requires: python3dist(pyusb)
> Requires: python3dist(pyserial)
is wrong.

In guidelines I see:
> These Provides tags can be used to list Requires and BuildRequires of a package using the standardized names
> (i.e. dist name, name on PyPI) of Python modules. To make it easier, you can use the %{py3_dist} macro that
> accept one or more parameters: the standardized name(s) of the desired Python software. It will convert the
> name(s) to the canonical format and create the proper python3dist(...) tag(s).

So I updated to:
> Requires: %{py3_dist pyusb}
> Requires: %{py3_dist pyserial}

Secondly:
> BuildRequires: python3-devel
> Requires: python3
> Requires: %{py3_dist pyusb}
> Requires: %{py3_dist pyserial}

I moved them inside %package (after summary).
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_example_python_spec_file


Here is the updated spec: https://gist.githubusercontent.com/luigifab/e2f77892bfc27185571d3731be4e422f/raw/383301a771f50556d4b77a4d8c4899a057c70d65/python-radexreader-fedora.spec
Here is the updated Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=65382127
And the updated SRPM: https://kojipkgs.fedoraproject.org//work/tasks/2135/65382135/python-radexreader-1.1.0-1.fc35.src.rpm

Updated to 1.2.0, spec url: https://raw.githubusercontent.com/luigifab/python-radexreader/master/fedora/python-radexreader.spec

Thanks for the fixes and sorry for the delay. In order to get this package approved, you need you request the repository as mentioned here: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

Since you've been sponsored already, I'm moving this to POST.

Sorry, but I can't request-repo, I getting the following error:
Could not execute request_repo: The Bugzilla bug's review was approved over 60 days ago

Up? :) sorry.