https://bugzilla.redhat.com/show_bug.cgi?id=2132701 --- Comment #6 from Petr Pisar <ppisar@xxxxxxxxxx> --- > c) Assume the line "# ??? psycopg2" is in the spec file and not "Requires: python3-psycopg2" because the package is only available for EPEL https://packages.fedoraproject.org/pkgs/python3-psycopg2/python3-psycopg2/ - if so, maybe Python RPM should not be built initially? The package seems to be needed for src/python/location/database.py though tests seem to pass. The comment is a remnant of a notice to myself the check whether the dependency on psycopg2 is needed. The dependency is needed. I will remove the comment. The dependency is expressed with "python3.11dist(psycopg2)". The dependency is satisfied in Fedora 38. Yes, the dependency is not used when running the tests. It's noticed in "# Run-time" section if BuildRequires. > d) Python package has examples/python as documentation. Should doc/private-key.pem doc/public-key.pem also be included in documentation? Those are cryptographical keys. I think keys should not be in a documentation. Keys should be unique to a user. I left them out intentionally. It minimizes a chance that someone will use them in a production. > e) Why differentiate between %define and %global for database location? There are subtle differences between global and define. It does not matter here. I will changed it into %global the guidelines recommends it <https://docs.fedoraproject.org/en-US/packaging-guidelines/#_global_preferred_over_define>. > f) Have not checked German locale handling It's just a translation of program messages into German language. > g) psycopg3 is also available https://www.psycopg.org/ - maybe worth checking if upstream will upgrade to this I will tell it to the upstream and point to <https://www.varrazzo.com/blog/2020/11/07/psycopg3-adaptation/>. > h) Perhaps the description can be updated to "determine the approximate physical location corresponding to a requesting ip address", which is probably more accurate than "location of someone on the internet" I took the summary from the upstream. I will rephrase it to speak about an IP address rather than about someone. I do not want to use "physical" location since the database returns country codes and autonomous system numbers. It does not speak geographical coordinates. Maybe "locality" could be a better word. An updates spec file is available on the same address. -- 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=2132701 _______________________________________________ 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
