https://bugzilla.redhat.com/show_bug.cgi?id=2115102
--- Comment #22 from Wayne Sun <gsun(a)redhat.com> ---
(In reply to Miro Hrončok from comment #11)
First of all, I am looking at
https://raw.githubusercontent.com/RedHatQE/pylero/main/pylero.spec -- I
understand from the RUL that this spec file lives in the upstream repository
at
https://github.com/RedHatQE/pylero/blob/main/pylero.spec and that you've
been updating it there when receiving feedback from Adam. This looks like a
classic example of downstream == upstream, which makes some things easier
but it can also create some friction. I recommend reading this first:
https://docs.fedoraproject.org/en-US/packaging-guidelines/
#_spec_maintenance_and_canonicity
If you are OK with that, it's fine.
For the downstream I would like the spec host on Fedora git which for my
understanding is:
https://src.fedoraproject.org/
While for request a repo on fedora git with:
$fedpkg request-repo pylero 2115102
Could not execute request_repo: The Bugzilla bug is not approved yet
So it only could be proceed when the bug got approved.
I've updated the upstream spec:
https://github.com/RedHatQE/pylero/blob/main/pylero.spec
which I'll reply inline to confirm what have changed.
===============
Next, I read the spec file line by line and will note down anything that I
do not understand or consider bad practice. Note that there are the
packaging guidelines and I also have opinions :) I will try to make it clear
if I don't like something personally (and I will explain why) or if
something is specified in the guidelines. It's completely fine to disagree
witch my opinions, but I would appreciate if you could explain why when you
choose to do that.
1) [OPINION] The values (right side of : after Name, Version, etc.) are not
aligned to the 16th column. That makes the specfile different from the one
listed at
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
#_example_spec_file and in my opinion, harder to read. There is however no
guideline to align to the 16th column. I just consider it the de-facto
standard at least for Python packages.
Updated, the 16th column does look better.
2) [OPINION] Unlike the example Python specfile, this specfile uses
"Source0:" instead of "Source:". Looking at the conversation here,
this was
changed from "SOURCE:" to "Source0:" based on Adam's comment #1.
While I
agree that "SOURCE:" is not typical at all, I disagree that the number 0
should be there. Many packages use it as it was the standard until quite
recently, but the packaging guidelines were updated to not use such numbers
were not necessary in
https://pagure.io/packaging-committee/pull-request/1157 -- I highly
recommend watching this talk from DevConf.CZ 2020 titled "Still packaging
like it's 1999?":
https://youtu.be/0bfA0441dxc?t=666 -- it explains the
numbers and other things. There is however no guideline to not using the
redundant number. I just consider redundant things from the previous decade
a tech debt.
Updated
3) [MUST NOT] There is "BuildRequires: python3-setuptools" in the spec file,
but the spec file uses %pyproject_buildrequires and the dependency is
generated from
https://github.com/RedHatQE/pylero/blob/0.0.4/pyproject.toml#L2 -- hence the
manual BuildRequires tag is redundant and the guidelines explicitly say:
"Automatically determined dependencies MUST NOT be duplicated by manual
dependencies." in
https://docs.fedoraproject.org/en-US/packaging-guidelines/
#_package_dependencies
The python3-setuptools is removed as it's not required.
4) [OPINION] The %description uses Markdown. The descriptions of RPM
packages are not explicitly formatted with any markup. Packagers get
creative sometimes but the content will always be rendered as plaintext. I
consider using Markdown (other than *s for emphasis and similar "easy"
constructs) confusing. Especially when the description starts with a #-style
heading. I am also not entirely sure "Welcome to Pylero" is a typical
description. I'd start with "This is Pylero". There is however no
guideline
to not using Markdown or to greet users in descriptions. I just consider
both highly rare.
Updated per recommendation.
5) [CONFUSION] The setuptools_scm dependency: I understand the
comment, but
I don't understand the rationale. *Why* is it removed? Is it just because we
can live without it and we want to make the dependency footprint as small as
possible? Or does the package build wrongly when setuptools_scm is
installed? If this is the footprint case, I'd drop it entirely, as it makes
the specfile a tiny bit more complex than necessary for a very little
benefit. If this is the second case, it needs to be explicitly mentioned in
the comment. It might also require a BuildConflicts tag in that case.
Updated with not change the setuptools_scm in spec
6) [CONFUSION] Why "rm -f %{buildroot}%{_bindir}/pylero"? What is the
rationale for not shipping the file? Adam says that if you are sure you
don't want the file, you should rm it instead of using
%_unpackaged_files_terminate_build. I completely agree with that, but I
don't understand why do you want to get rid of it. Once we know, the reason
should IMHO be documented ina comment. It is also my opinion that spec file
should not use rm -f. If the file is no longer there, the rm should be
removed from the specfile -- but with rm -f you will never notice.
So the pylero CLI is not recommended for fedora as it prefer gnureadline
package (non-fedora release) over native python3 readline package, this should
not be an issue for packaging, I have removed the 'rm' part.
7) [OPINION] The %files section does not use %{pyproject_files} (and
the
%install section does not use %pyproject_save_files). I consider using this
the best practice, but there is no guideline that would say this is a MUST
or SHOULD.
Updated with using pyproject_files
8) [SHOULD NOT] The %changelog is quite verbose for downstream. Usually,
downstream changelog entries say "Updated to 0.0.4" or similar. No need to
reuse the upstream changelog here. See
https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs
"""Changelog entries should provide a brief summary of the changes done
to
the package between releases, including noting updating to a new version,
adding a patch, fixing other spec sections, note bugs fixed, and CVE’s if
any. They must never simply contain an entire copy of the source CHANGELOG
entries. The intent is to give the user a hint as to what changed in a
package update without overwhelming them with the technical details. Links
to upstream changelogs can be entered for those who want additional
information."""
The changelog haven't been updated current spec file is upstream and downstream
in fedora git will remove it.
For issue 9:
I have updated the package name to python-pylero as it's preferred to be used
as library.
For issue 10:
Yeah, I have added the %pyproject_check_import under %check
Thanks for reviewing and explain all the details.
--
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=2115102