https://bugzilla.redhat.com/show_bug.cgi?id=2250532 Sandro <gui1ty@xxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ --- Comment #8 from Sandro <gui1ty@xxxxxxxxxxxxx> --- (In reply to Ben Beasley from comment #4) > Thanks for the review! > > (In reply to Sandro from comment #3) > > [x]: License file installed when any subpackage combination is installed. > > > > => mrs_tools-1.0.2-1.fc40.noarch.rpm does not contain a license file. I see > > that it requires python3-nifti-mrs, which has a license file. > > Personally, I'd err on the side of caution here. Not blocking this review, > > but rather something to keep in mind. > > The guidelines are very explicit that a dependency on a subpackage with the > correct license file is sufficient[1]. I agree it’s important to make sure > we don’t break the dependency without handling the license file, but I don’t > think it’s worth duplicating the license file, and I haven’t found doing so > to be the usual practice. The reason I was more alerted than usual is that `mrs_tools` and `python3-nifti-mrs` are separate packages and I checked them separately. As long as one depends on the other all is well, indeed. Regarding duplicate license files, I know plenty of packages adding the license file separately to every sub package, where I would prefer having it in the main package and the sub package requiring the main package. Different packagers, different styles. > > [!]: Latest version is packaged. > > > > => Version 1.1.1 was released on December 6, 2023. Please update before > > importing. > > Yeah, I should have double-checked this before calling for a reviewer. I’ll > post an updated submission. The changes are not disruptive[2], so I don’t > expect there will be anything different in the packaging. > > > => A nitpicky bit: Consider standardizing on BRs. You are using > > `python3-foo` and `%{py3_dist foo}` intermixed. > > I agree with standardizing in general, but in this case there is a good > reason for each python3-foo: > > - For python3-nifti-mrs, this is a fully-versioned dependency across > subpackages of the same package in the manner of [3], so it should use the > exact binary package name to avoid any ambiguity or versioning discrepancies > (like with Python Provides dropping trailing .0’s, while they’re maintained > in the RPM package version). > > - For python3-sphinx-latex, this is a metapackage that brings in the > dependencies for building LaTeX with Sphinx; it does not contain any > importable package and specifically does not Provide > python3dist(sphinx-latex). > > So I’ve standardized on using %{py3_dist foo} everywhere that I can – for > all external dependencies on importable Python packages - and this package > just happens to contain only one such dependency, %{py3_dist pytest}, along > with both of the notable exceptions where python3-foo doesn’t really mean > %{py3_dist foo}. I'm not quite sure I fully understand your point here. Maybe that's because I'm used to specifying dependencies as `python3-foo`, in which case it doesn't matter if the package is a meta package or an actual Python package. For Python packages that do provide importable modules, is there a difference in specifying them as `python3-foo` or as `%{py3_dist foo}`. If not, then that would be reason enough for me to standardize on the `python3-foo` notation. Of course, if your preference is with `%{py3_dist foo}`, you end up having to mix both as in the case of `python3-sphinx-latex`. > > => Bonus points for handcrafted man pages! > > Get your hand-crafted, artisanal man pages right here! Only fifteen dollars > a sheaf! Join our mailing list and get 10% off! 🤑🤑🤑 (In reply to Ben Beasley from comment #5) > I will also change standardised to standardized in the description so that > it is written in American English[1]. > > [1] > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_summary_and_description I'll add more bonus points. You now have 99 bonus points. You require 101 for a washing machine [1]. Package is APPROVED! [1] As per our terms and conditions, 100 bonus points is the maximum per review and bonus points cannot be carried over. -- 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=2250532 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202250532%23c8 -- _______________________________________________ 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
