https://bugzilla.redhat.com/show_bug.cgi?id=2388375 Cristian Le <fedora@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@xxxxxxxxxxxxxxxxx |fedora@xxxxxxxxx --- Comment #13 from Cristian Le <fedora@xxxxxxxxx> --- Great, now to iron out more details of the review: - Please use explicitly the format ``` Spec URL: ... SRPM URL: ... ``` This is used both by fedora-review bot, and other post-review automations. You can freely add any other lines before and after - Are you already a packager or do you need sponsorship? If the latter, add `FE-NEEDSPONSOR` to the blocks of this bug - If my bash-foo doesn't fail me, the ` || :` means that you are ignoring the return code of the commands. Please let it run and fail if needed, especially in order to detect changes later on - The project version in source [1] and the release is 0.5.1, please also use that - If you are going to use a commit snapshot, see the snapshot guideline [2]. Forgemeta can automate this, e.g. here is one of mine that uses it [3] (%forgesource0 and %forgeautosetup is also relevant there) - Consider using autospec macros (%autorelease and %autochangelog). It is not a blocking issue because it is not a requirement and some people do not like using it, but it is a very useful approach especially for others contributing PRs to the dist-git repo - The line `# Remove any wrongly-installed paths under sitearch/usr (happens with old setuptools)` would need more context (here in the review is enough). What files are in there? If relevant, preferably change the `setup.py` to install to `/usr` instead of `usr` and it should land in the correct spot? This part [4] suggests that it should already install in the correct location though. - Please use `%pyproject_save_files`, and try to add `-l` to it. If it works, then you can remove the `%license` from `%files` since it would be done automatically. `%{python3_sitearch}/...` etc. is already covered by `%pyproject_save_files` - The `BuildRequires: py3_dist` should already be handled by the `%pyproject_buildrequires` - The project seems to be bundling and have some changes on top of the gnushogi engine right? In that case you should add a `Provides: bundled(gnushogi)`. If it doesn't call the `gnushogi` cli (which I didn't see in the repo?), you can remove the the `Requires: gnushogi` - %description lines must be wrapped to 80 character length - Can you add a `%pyproject_check_import` to the `%check` - Do you know how the `/locale` folder is used? - Can you remove `gshogi/data/opening.bbk` and create it manually with `create_book.py` and similarly for the other generated files? Given that this project is effectively dead, we can use the pre-compiled ones though, after checking if there is any significant difference if we run them manually now. But if this project is resurrected, you should be re-generating all files manually. - Feel free to ping me (@lecris:matrix.org) in the #devel chat room if you have quick questions and such That is all I have for now. I still need to run the licensecheck to double check that everything is alright, but at a glance I do not see anything problematic. I assume that all the images are public domain and not copyrighted. [1]: https://github.com/johncheetham/gshogi/blob/7c4bd90199c5bda61984347ea68c32521e82a637/gshogi/constants.py#L21 [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots [3]: https://src.fedoraproject.org/rpms/span/blob/rawhide/f/span.spec#_3-11 [4]: https://github.com/johncheetham/gshogi/blob/7c4bd90199c5bda61984347ea68c32521e82a637/setup.py#L31-L33 -- 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=2388375 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202388375%23c13 -- _______________________________________________ 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
