https://bugzilla.redhat.com/show_bug.cgi?id=1982635 Otto Urpelainen <oturpe@xxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ce@xxxxxxx) --- Comment #1 from Otto Urpelainen <oturpe@xxxxxx> --- Reviewed. This is my first php review, so please giv guidance if some of my comments are misguided. Two blocking issues and some non-blocking found: 1. > License: GPLv3+ Both the LICENSE file and composer.json say it is actually MIT. Please fix. 2. >From fedora-review: Note: Directories without known owners: /usr/share/php/League This is a MUST item in the guidelines, so it must be fixed. I am not sure how vendor directory ownership is usually handed in php packages. I suppose all packages from League could share the ownership? Then this would do it: %dir %{_datadir}/php/League Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership > # Please preserve the changelog entries I do not understand this request in combination with %autochangelog 3. Guidelines say "The composer.json file is not used, and should be installed as %doc as it provides useful information about the package and its dependencies.". I am not certain how to interpret this statement. Since it says should, I do not think it is a blocking issue. However, it clearly recommends installing the file as %doc instead of dropping it completely like is done here. Perhaps it could be added as %doc as suggested? Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/PHP/#_file_placement 4. > : No tests implemented This one is strange. There is a dev dependency to phpunit in composer.json, there is phpunit.xml whose syntax is do not completely understand but I think it is trying to say "execute everything from src/ that ends with Test.php" and there are two such files under src/Idna. But upstream removes them from the Git archives via .gitattributes statements. Since the guidelines say test suites should be run, it would be good to contact upstream about how to make running the tests possible. This is not a blocking review issue, because running tests is just a should and upstream has intentionally stripped them. In any case, the comment should describe this situation. -- 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
