https://bugzilla.redhat.com/show_bug.cgi?id=1987045 Troy Curtis <troy@xxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(stuart@xxxxxxxxxx | |g) --- Comment #4 from Troy Curtis <troy@xxxxxxxxxxxxxxxx> --- Great pointers Ben! In addition to Ben's feedback, I had a few additional suggestions and questions. I'll save posting fedora-review output until the next round since my output is similar to Ben's. Questions: - Should the `*` be removed from the license text? - Should the slightly more restrictive BSD 3-clause found in anet.h be used instead? Small issues: - the html page does not need execute perms, suggest 644 instead. - Remove the commented lines for Requires & configure - Can you do a scratch build so we can see how this package behaves on other archs? I followed the directions in the README and was able to get between 6-8 tracks using my little whip antenna almost immediately, so it seems to works as described! If you do decide to create and maintain the manpage, it might be nice to put the few quick examples outlined in the README there. I found them to be really helpful. Personal opinion on the patch. The patch is arguably specific to this package, so I would be surprised if it makes sense to upstream. Still there should be a comment on the patch explaining that. -- 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 _______________________________________________ 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
