search

OCA / geospatial

Odoo and GIS
http://oca.github.io/geospatial/index.html
GNU Affero General Public License v3.0
189 stars 284 forks source link

[12.0][FIX] Name and order of parameters of the gist index creation function #308

Closed sersanchus closed 1 year ago

sersanchus commented 2 years ago

It's been a while since spatial indexes could not be created on geographic fields. This commit correctly uses the nomenclature of the functions and the parameters to use.

yvaucher commented 2 years ago

/ocabot merge patch

OCA-git-bot commented 2 years ago

This PR looks fantastic, let's merge it! Prepared branch 12.0-ocabot-merge-pr-308-by-yvaucher-bump-patch, awaiting test results.

OCA-git-bot commented 2 years ago

@yvaucher your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-308-by-yvaucher-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

sersanchus commented 2 years ago

There are some coverage tests that fail but have nothing to do with this fix. The merge must be accepted anyway.

yvaucher commented 2 years ago

The coverage shouldn't block the merge

yvaucher commented 2 years ago

/ocabot merge patch

OCA-git-bot commented 2 years ago

On my way to merge this fine PR! Prepared branch 12.0-ocabot-merge-pr-308-by-yvaucher-bump-patch, awaiting test results.

yvaucher commented 2 years ago

The error was 

----------

FAIL

FAIL: test_geo_localize (odoo.addons.geoengine_base_geolocalize.tests.test_geoengine_partner.TestGeoenginePartner)

Traceback (most recent call last):

`   File "/home/travis/build/OCA/geospatial/geoengine_base_geolocalize/tests/test_geoengine_partner.py", line 35, in test_geo_localize

`     "Latitude Should be equals",

` AssertionError: 49.95506 != 49.95353 within 5 places : Latitude Should be equals

FAILED

Module geoengine_base_geolocalize: 1 failures, 0 errors

At least one test failed when loading the modules.

Message not found: 'Modules loaded.'

I doubt the index should have any impact. I relaunched the merge to check if the issue persists.

OCA-git-bot commented 2 years ago

@yvaucher your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-308-by-yvaucher-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

sersanchus commented 2 years ago

That is the coverage test problem I commented before, the call to 'create_geo_index' is not covered by any unit test...

Check warning on line 148 in base_geoengine/fields.py

Codecov / codecov/patch base_geoengine/fields.py#L148 Added line #L148 was not covered by tests

yvaucher commented 1 year ago

@sersanchus The code coverage should not be a blocking check, it seems the CI failed on the tests with the ocabot: https://github.com/OCA/geospatial/runs/8135928245

yvaucher commented 1 year ago

/ocabot merge patch

OCA-git-bot commented 1 year ago

What a great day to merge this nice PR. Let's do it! Prepared branch 12.0-ocabot-merge-pr-308-by-yvaucher-bump-patch, awaiting test results.

yvaucher commented 1 year ago

Trying a new merge now that we use github actions

OCA-git-bot commented 1 year ago

Congratulations, your PR was merged at 5da4bfd12617735b790db5914cef73dd63aa4ac8. Thanks a lot for contributing to OCA. ❤️