Spec URL: https://anthr76.fedorapeople.org/python-dotref/python-dotref.spec SRPM URL: https://anthr76.fedorapeople.org/python-dotref/python-dotref-1.2.4-1.fc37.src.rpm Description: A simple tool to manage dotfiles across multiple devices. It supports creating directories, symlinks, templating and hierarchical configuration profiles to keep things DRY. Fedora Account System Username: anthr76
Here's some drive by feedback: There's new macros for shell completions, and all of the completions directories are owned by filesystem. You can apply this patch[1] to your specfile and see [2] for more information. [1]: https://paste.sr.ht/~gotmax23/ad16ede10a059f561e61ce348dd38eb3abc7a22f [2]: https://pagure.io/packaging-committee/issue/1202 You reference %{pypi_name}, but that doesn't seem to be defined anywhere in your specfile. Instead of defining that macro, I'd suggest replacing all instances of `%{pypi_name}` with the literal value. ``` # Remove bundled egg-info rm -rf %{pypi_name}.egg-info ``` There's no dotref.egg-info file in the archive, and this wouldn't be necessary even if there was. > Source: %{url}/archive/refs/tags/v%{version}/dotref-%{version}.tar.gz The preferred format is %{url}/archive/v%{version}/dotref-%{version}.tar.gz, but meh --- It would be nice if you provided a link to a COPR or Koji scratch build.
Hi Max thanks for your remarks! > There's new macros for shell completions, and all of the completions directories are owned by filesystem. You can apply this patch[1] to your specfile and see [2] for more information. This is great information. I didn't know thank you. > You reference %{pypi_name}, but that doesn't seem to be defined anywhere in your specfile. Instead of defining that macro, I'd suggest replacing all instances of `%{pypi_name}` with the literal value. Ah yes. I originally tried this with pyp2rpm but realized some of the generation is with the old style macros. > There's no dotref.egg-info file in the archive, and this wouldn't be necessary even if there was. Good point. > The preferred format is %{url}/archive/v%{version}/dotref-%{version}.tar.gz, but meh I'm a fan of conventions +1 Copr Build: https://copr.fedorainfracloud.org/coprs/anthr76/dotref/build/4897386/ Spec URL: https://anthr76.fedorapeople.org/python-dotref/python-dotref.spec SRPM URL: https://anthr76.fedorapeople.org/python-dotref/python-dotref-1.2.4-1.fc37.src.rpm
Howdy Anthony, I recognized your name from Twitter. I can take this review. Based on a quick skimming of the upstream README, this appears to be primarily an application, not a library. Per the guidelines [0] it should be named dotref, not python-dotref. Making that change will also have the pleasant side effect of no longer needing the python3-dotref subpackage, the %_description macro, or the `-n dotref-%{version}` flag for %autosetup. What's the reason for using pytest to run the tests? Upstream uses coverage [1], which we should avoid [2], but a simpler approach that avoids the additional build requirement would be to use `%{python3} -m unittest -v`. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_application_naming [1] https://github.com/ovk/dotref/blob/v1.2.4/Makefile#L5 [2] https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_linters
The %pytest macro runs the tests against the code we actually ship (i.e. what's installed in %{buildroot}%{python3_sitelib}) instead of the source tree, so it's preferable to use that as the test runner.
That's a fair point, but it can also be accomplished by setting an environment variable, which doesn't involve an unnecessary build requirement. PYTHONPATH=%{buildroot}%{python3_sitelib} %{python3} -m unittest -v
This is an automatic action taken by review-stats script. The ticket submitter failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we consider this ticket as DEADREVIEW and proceed to close it.