https://bugzilla.redhat.com/show_bug.cgi?id=2132701



--- Comment #6 from Petr Pisar <[email protected]> ---
> 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 -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue

Reply via email to