search

datagouv / edigeo-parser

Blazing fast parser for EDIGEO files
MIT License
8 stars 3 forks source link

Es6 migration #26

Closed ThomasG77 closed 1 year ago

ThomasG77 commented 1 year ago

Needed to introduce JSTS for fixing some geometries issues at #22 level. Main concern is about https://github.com/etalab/edigeo-parser/pull/26/files#diff-ca900686fca4680d4692bf587dc5da1322879c344688c187b5511cda27902c18 I use import {parse} from '../lib/index.js' whereas when porting I would expect the cli would work when using import {parse} from '../index.js'

See gist https://gist.github.com/ThomasG77/15cedc21e23744461191df6ccd4d7b6b for more detailled.

Ping @jdesboeufs

jdesboeufs commented 1 year ago

Hello Thomas,

I think It’s a bad idea to migrate this module for now. You should migrate etalab/cadastre first or you will have to transpile or stay on a old version of edigeo-parser.

For the geometry problem, consider do a separate PR or you will be unable to investigate regressions. 1 PR = 1 purpose

ThomasG77 commented 1 year ago

Already working on etalab/cadastre repo e.g to use this es6 repo e.g https://github.com/etalab/cadastre/tree/es6-migration Uncomplete at the moment as it also depends in particular on another CommonJS project https://github.com/etalab/edigeo-reproject

For the geometry problem, consider do a separate PR or you will be unable to investigate regressions.

This PR only purpose is to migrate to es6. I do not introduce jsts or any other corrections. I've just explained why I'm doing the upgrade to es6: to be able to introduce jsts. I've some standalone geometry corrections in individual scripts that I do not introduce here and will only introduce after es6 migration.

All tests and linting command passed but the circleci pipeline is broken so not showing here...