https://bugzilla.redhat.com/show_bug.cgi?id=2394287 --- Comment #2 from Carl George 🤠 <carl@xxxxxxxxxx> --- Howdy Scott, I can help with this review. Thanks for bringing it up on the epel-devel mailing list. ================================================================================ The first thing that jumps out at me is the package naming. Based on the upstream README, this seems to be primarily an application, not a library. Therefore the name of the package should be just vcs2l, not python-vcs2l. This will also allow you to remove the subpackage sections, avoid the need to create a %_description macro, and simplify %autosetup (no need to pass the -n flag). https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_application_naming ================================================================================ This package should also utilize %autorelease and %autochangelog. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_release_tag https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs ================================================================================ I don't think you actually want to use %{python3_pkgversion} in this spec file. That is really only useful when the same package needs to be built for multiple Python versions. Since this is an application and not a library, I imagine you'll only ever want to ship a single version built against the default Python version. The spec file legibility can be improved by dropping these, e.g. python%{python3_pkgversion}-devel -> python3-devel https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_system_settings https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility ================================================================================ Currently this software just has one build requirement (pytest), but that could change in the future. Consider using the automatic buildrequirement generator so that in the future any additional build requirements get accounted for. Upstream makes this easy by defining those in a "test" extra. %pyproject_buildrequires -x test https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#Automatically-generated-dependencies ================================================================================ It isn't useful to ship shell completions in /usr/share/vcs2l-completion. These should be used to the standard locations (with standard names) so they are utilized by the respective shells. The bash, fish, and zsh completions are covered by the packaging guidelines. I don't know what the right path and naming convention is for tcsh. I'll leave it up to you if you want to figure that out or just delete that file. https://docs.fedoraproject.org/en-US/packaging-guidelines/ShellCompletions/ -- 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=2394287 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202394287%23c2 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
