https://bugzilla.redhat.com/show_bug.cgi?id=2016661 --- Comment #4 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- Thanks for the review! (In reply to Maxwell G from comment #3) > Your specfile looks great! Thank you for putting in the time to figure out > the documentation. I just have a couple minor notes/questions. > > Notes > ===== > > > %package doc > > Summary: Documentation for geomdl > > # From docs/citing.txt: > > # * Source code is released under the terms of the MIT License > > # * Examples are released under the terms of the MIT License > > # * Documentation is released under the terms of CC BY 4.0 > > License: CC-BY > > The MIT-licensed examples are not included in the docs package, correct? Correct, those are in a separate repository[1] that is not packaged. I’ll modify the comment to mention that fact. [1] https://github.com/orbingol/geomdl-examples > Also, docs/citing.txt should be docs/citing.rst. Good catch. I will fix that. > I would add a weak dependency on the doc subpackage in the python3 > subpackage so that it's more discoverable. That's really up to you. I appreciate the suggestion. I’d prefer not to add a weak dependency (Recommends:) because if I do, the documentation will be pulled in whenever the python3-geomdl package is installed unless the user specifically asks not to (dnf --setopt=install_weak_deps=False). I think that defeats most of the purpose of separate documentation packages—noting that the documentation is (packed) twice the size of the library, and considering that this will happen even if the package is installed only as a dependency. I’m happy to add a hint (Suggests:), although I’m not sure if any widely used package management tool actually uses those. > It looks like you are including the license in both subpackages and not > having the doc subpackage depend on the python3 subpackage to allow users to > install one but not the other. > > > BuildRequires: %{py3_dist pytest} >= 3.6.0 > > Is there a reason you use the `%py3_dist` macro here and use py3dist() > everywhere else? I know that `%py3_dist` expands to `py3dist(NAME)`, but you > should probably remain conistent. Upstream provides a tox env that you can > use if you prefer. No, there’s no reason. I would rather use python3dist() everywhere. I revisited this and observed that %tox does work quite well in this package—sometimes it’s a struggle to get it to do *only* what is needed for an RPM build, and direct pytest invocation is easier—so I’ll just switch to that, saving the explicit pytest BR. -- 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=2016661 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
