[12.0] [IMP] geoengine_swisstopo : Add Swisstopo projections #351
Conversation
sbrunner
force-pushed
the
swisstopo-projections
branch
16 times, most recently
from
00d1758
to
e691044
Compare
There was a problem hiding this comment.
for the code, LGTM :)
but for the 3 commits, i'm not sure if you shall split them in 2 (pre-commit and swisstopo projections)
same here for the name of the PR. i'm not sure if shall be rename to contain the version, type of pr (implementation) and a title explicit about the module that contain most of the modification and a description of the implementation. example :
[12.0] [IMP] base_geoengine : Adding Swisstopo projections
@yvaucher could you confirm or refute this :))
There was a problem hiding this comment.
Looks great, just have a little concern on the multiple odoo.define for projections as it seems redundant.
Plus about the commits, some part of the changes of the 2nd commit deserve to be in the 3rd commit (or be squashed) the linting could go in a separate commit to let your last commit emphasis on the practical changes.
|
|
||
| var PROJECTION_CODE = "EPSG:21781"; | ||
| const GeoengineWidgets = require("base_geoengine.geoengine_widgets"); |
There was a problem hiding this comment.
The require is similiar as an import wouldn't it make more sense to keep it at the root level of the file or was there some cyclic import issue?
There was a problem hiding this comment.
I just keep it as it was, and the require function is not defined at top level...
yvaucher
changed the title
yvaucher
changed the title
yvaucher
changed the title
sbrunner
force-pushed
the
swisstopo-projections
branch
from
e691044
to
7f272e0
Compare
sbrunner
force-pushed
the
swisstopo-projections
branch
from
7f272e0
to
82f6a43
Compare
Use the service Capabilities
sbrunner
force-pushed
the
swisstopo-projections
branch
from
82f6a43
to
3f3f687
Compare
|
/ocabot merge minor |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at e8e6b94. Thanks a lot for contributing to OCA. ❤️ |
| @@ -131,7 +131,7 @@ def get_edit_info_for_geo_column(self, column): | |||
| 'default_extent': view.default_extent or DEFAULT_EXTENT, | |||
| 'default_zoom': view.default_zoom, | |||
| } | |||
| logger.debug("Parameters for geo field {}:\n{}".format(column, res)) | |||
| logger.debug(f"Parameters for geo field {column}:\n{res}") | |||
There was a problem hiding this comment.
the minimum version of python supported by v12 is 3.5, which does not have fstrings. This unfortunately broke my installation.
| - repo: https://github.com/myint/autoflake | ||
| rev: v1.4 | ||
| hooks: | ||
| - id: autoflake | ||
| args: | ||
| - --expand-star-imports | ||
| - --ignore-init-module-imports | ||
| - --in-place | ||
| - --remove-all-unused-imports | ||
| - --remove-duplicate-keys | ||
| - --remove-unused-variables | ||
| # - repo: https://github.com/psf/black | ||
| # rev: 22.3.0 | ||
| # hooks: | ||
| # - id: black | ||
| # - repo: https://github.com/pre-commit/mirrors-prettier | ||
| # rev: v2.1.2 | ||
| # hooks: | ||
| # - id: prettier | ||
| # name: prettier (with plugin-xml) | ||
| # additional_dependencies: | ||
| # - "[email protected]" | ||
| # - "@prettier/[email protected]" | ||
| # args: | ||
| # - --plugin=@prettier/plugin-xml | ||
| # files: \.(css|htm|html|js|json|jsx|less|md|scss|toml|ts|xml|yaml|yml)$ | ||
| - repo: https://github.com/asottile/pyupgrade | ||
| rev: v2.7.2 | ||
| hooks: | ||
| - id: pyupgrade | ||
| args: | ||
| - --keep-percent-format | ||
| - --py36-plus | ||
| # - repo: https://github.com/PyCQA/isort | ||
| # rev: 5.12.0 | ||
| # language_version: python3.8 | ||
| # hooks: | ||
| # - id: isort | ||
| # name: isort except __init__.py | ||
| # args: | ||
| # - --settings=. | ||
| # exclude: /__init__\.py$ |
There was a problem hiding this comment.
This shouldn't be modified manually, but only through copier update
|
Fixed the issue in #366 |
No description provided.