https://bugzilla.redhat.com/show_bug.cgi?id=2060621 Jakub Kadlčík <jkadlcik@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value CC| |jkadlcik@xxxxxxxxxx --- Comment #1 from Jakub Kadlčík <jkadlcik@xxxxxxxxxx> --- Hello Robert, thank you very much for the package. Overall, the spec file looks very good, but there is a couple of things that we need to fix. > Spec URL: <https://github.com/ryanjohnmccann/capstone-2021-22/blob/main/python-abstract.spec> > SRPM URL: <https://github.com/ryanjohnmccann/capstone-2021-22/blob/main/python-abstract-1-1.src.rpm> It's a bit unusual to have the < pointy brackets > there, but it's okay. However, the links need to point directly to the raw file / file download. In the case of the spec, you want to go to the link you posted, click the "Raw" button, and use that URL. For the SRPM, you want to "Copy link address" for the "Download" button and use that. There are tools that we use for the review, that download those files, so they need direct links. > Summary: Python library for creating and drawing graphs and taking advantage of graph properties I think there is no such rule (packaging guidelines don't say so) but generally, it is a good idea to make the summary at max 80 characters long. When you imagine searching packages in GUI package managers or in DNF, they typically show package name and summary on the same line, also people often don't have the window maximalized, so we want to display all the information in some reasonable width. I would probably drop the "python library for", that's obvious from the package name and change the verbs from -ing form to their simple form, e.g. "Create and draw graphs and take ...". Just a suggestion, we can go with anything else that works for you better. > Prefix: %{_prefix} Does some tutorial recommend this? I believe this is a historical thing. I think you can safely remove it now. > %description > Abstract is a Python library for creating and drawing graphs > and taking advantage of graph properties. This is basically a copy-pasted summary, we try to avoid that. Can you please write a few sentences describing the package, what it is good for and what it can do? There is a lot of text in the project README, I think we can condense it into a short description paragraph. > %build > %py3_build The package builds correctly on your system because you already have some python dependencies installed, but if you try to build it in a minimal chroot (that's how it is going to be done in Fedora), it fails with + %py3_build /var/tmp/rpm-tmp.GUgMDY: line 42: fg: no job control error: Bad exit status from /var/tmp/rpm-tmp.GUgMDY (%build) That's because of a missing BuildRequires for python3-devel. It is a good idea to build your package in Copr or Mock, they will reveal all the missing BuildRequires that you forgot. Copr - https://docs.pagure.org/copr.copr/screenshots_tutorial.html Mock - https://fedoraproject.org/wiki/Using_Mock_to_test_package_builds#How_do_I_use_Mock? (Don't worry about the document length, using mock is quite simple. The only important section for you is the "How do I use Mock?") > %changelog > * Sat Feb 26 2022 Robert Santos <robert.santoshld2018@xxxxxxxxx> and Ali Dia <ali_dia@xxxxxxxxxxxxxxx> > * - First abstract package The changelog is in an unexpected format, please take a look here https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs I haven't seen any changelog entries by two authors at the same time, but I believe in giving people the credit that they deserve for their work. Maybe let's have only one of you in the changelog entry and appreciate the other in a comment above? (each line that starts with # is a comment) Also, the second line shouldn't start with *, see the link above. --- Sorry for a lengthy comment, in fact, all of those things are easy-fixes and overall the package looks really good. -- 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=2060621 _______________________________________________ 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
