https://bugzilla.redhat.com/show_bug.cgi?id=2264463 --- Comment #26 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- (In reply to Fabio Valentini from comment #23) > Package looks good to me. Full review below. Some notes: > > 0. Shipping Python metadata would be fine with me, reading up on previous > comments. But it could always be added later if it turns out to be needed > for something. I think I’ll stay the course for now, but if this Rust/maturin/PyPI executable pattern becomes popular, I suspect we will need to settle on a default approach that does package the Python metadata. > 1. The no-strip patch is no longer necessary since you no longer use maturin > for building. You could drop it - or keep it, if you think switching back to > maturin + shipping Python metadata might happen in the future, and want to > keep this "documented" until then. Hmm, good point. I think I’ll keep the patch, but add a comment documenting that it doesn’t matter when building directly with cargo. > 2. You might want to add "ExcludeArch: i686" since this is a new leaf > package, but that is your decision. This is a good idea, and I’ll do it. > 3. There is no manual page for the CLI application. I don't mind, but you > told me you like them. It looks like cramjam-cli uses clap for constructing > its CLI, so it should have "useful" help output that can be piped to > help2man :) I do like having man pages, but only ones that are useful. Unfortunately, help2man generates a terrible man page in this case: - The list of subcommands is poorly formatted and jumbled inline with the description of the “help” subcommand - The example is tacked onto the end of the OPTIONS section instead of placed in an EXAMPLES section - The -l/--level option is documented only in the subcommand help output, e.g. cramjam-cli snappy --help. - It’s not clear from the generated output that all of the subcommands take exactly the same options. Since it doesn’t look like the CLI options are going to change much (particularly, it doesn’t look like there will be low-level algorithm-specific options), so the maintenance burden won’t be too high, I may in fact use the --help output to hand-write a man page that is properly formatted and actually useful. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2264463 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202264463%23c26 -- _______________________________________________ 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
