search

postgrespro / pgsphere

PgSphere provides spherical data types, functions, operators, and indexing for PostgreSQL.
https://pgsphere.org
BSD 3-Clause "New" or "Revised" License
16 stars 15 forks source link

Added calculation of the distance between a line and a point #40

Closed Darya177777 closed 10 months ago

esabol commented 11 months ago

@Darya177777 : CI tests failed. Make sure your fork is sync'ed with this repo. You might need to just update the line numbers in some expected output files?

Darya177777 commented 11 months ago

@esabol , @vitcpp : All tests fixed. Please, re-review.

Darya177777 commented 11 months ago

All fixed.

esabol commented 11 months ago

All fixed.

OK, looking better! @vitcpp will probably have some comments as well.

vitcpp commented 11 months ago

@Darya177777 Could you also please rebase on the newest version and update the upgrade script upgrade_scripts/pg_sphere--1.2.3--1.3.0.sql.in. Such script is used when updating the extension to a newer version using ALTER EXTENSION UPDATE TO command. Once you have introduced a new function the upgrade script should be updated.

vitcpp commented 11 months ago

@Darya177777 Please, ignore this comment. It seems I was wrong when claimed these cases to be wrong.

@Darya177777 I've found one case when a distance seems to be wrong: image image Could you please check it?

vitcpp commented 11 months ago

@Darya177777 Could you please squash two commits into one.

vitcpp commented 11 months ago

PR seems to be ok. I need to do some testing before merge.

esabol commented 11 months ago

@Darya177777 Could you please squash two commits into one.

@vitcpp, doesn't GitHub automatically squash commits on merge these days? I've been under the impression that it does, but maybe I'm used to GitLab.

vitcpp commented 11 months ago

@esabol I'm not sure about github feature to squash commits. I will check it. I'm concerned which final comment will appear in commit after squashing. I'm not sure that simple joining of commit messages is a good idea. Furthermore, it is harder to review multiple commits where latest commit overwrites some changes of the previous commit. PR should contain the final version of the changes. It can contain multiple commits but these commits should be used to logically separate the changes. They should not touch the same code.

vitcpp commented 11 months ago

@Darya177777 Could you please fix the tests? Thank you.

vitcpp commented 11 months ago

@Darya177777 You have added sscan.c file to the commit. It shouldn't be added. Could you please remove it from the commit.

Darya177777 commented 11 months ago

@vitcpp , All fixed

esabol commented 11 months ago

@esabol I'm not sure about github feature to squash commits. I will check it. I'm concerned which final comment will appear in commit after squashing. I'm not sure that simple joining of commit messages is a good idea. Furthermore, it is harder to review multiple commits where latest commit overwrites some changes of the previous commit. PR should contain the final version of the changes. It can contain multiple commits but these commits should be used to logically separate the changes. They should not touch the same code.

@vitcpp, you can edit the merge commit message to whatever you want when squashing and merging, according to this: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits

vitcpp commented 11 months ago

@vitcpp, you can edit the merge commit message to whatever you want when squashing and merging, according to this: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits

@esabol Thank you very much! I will take a look.

Darya177777 commented 11 months ago

@vitcpp I have made some changes in pg_sphere--1.2.3--1.3.0.sql.in. Please, re-review.