Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(api): #2856 data integration corse #2899

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

feat(api): #2856 data integration corse #2899

wants to merge 5 commits into from

Conversation

461OceanBd
Copy link
Collaborator

  • Added headers path
  • Added excluded BOM conf when parsing

@461OceanBd 461OceanBd force-pushed the 2856-data-integration-corse branch 2 times, most recently from 3a9f909 to 481b903 Compare November 19, 2024 17:11
@461OceanBd 461OceanBd self-assigned this Nov 20, 2024
@461OceanBd 461OceanBd marked this pull request as ready for review November 21, 2024 09:07
@461OceanBd 461OceanBd linked an issue Nov 25, 2024 that may be closed by this pull request
Ajouter les données pour la région de la Corse #2856
Open
alice-telescoop
alice-telescoop reviewed Nov 25, 2024
View reviewed changes
Copy link
Collaborator

@alice-telescoop alice-telescoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

si la question posée ne pose pas pb je valide la PR

packages/api/src/modules/providers/scdl/scdl.grant.parser.ts Show resolved Hide resolved
@461OceanBd
Copy link
Collaborator Author

@mxmeunier tu avais des retours à me faire là dessus ?

@461OceanBd
Copy link
Collaborator Author

461OceanBd commented Nov 27, 2024 •
edited
Loading

@mxmeunier tu avais des retours à me faire là dessus ?

Après analyse, il n'y avait pas de trim() sur les headers des en-têtes autre part, c'est pour ça d'ailleurs que j'avais ce problème pour la Corse. J'ai quand même supprimé trois trim() qui étaient effectués sur les libellés des headers du SCDL_MAPPER. Inutile de se polluer avec ça. Je merge la PR.
Pas de changement finalement.

@461OceanBd
feat(api): #2856 data integration corse
92565b7 
- Added headers path
- Added excluded BOM conf when parsing
@461OceanBd
feat(api): #2856 data integration corse
7ef84fe 
- Added trim() to all csv headers (remove line break chars)
@461OceanBd
feat(api): #2856 data integration corse
8e632a5 
- Added columns callback tests
@461OceanBd
feat(api): #2856 data integration corse
94b4bbd 
- remove unnecessary trim
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-telescoop alice-telescoop alice-telescoop left review comments

@mxmeunier mxmeunier Awaiting requested review from mxmeunier

Assignees

@461OceanBd 461OceanBd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ajouter les données pour la région de la Corse
2 participants
@461OceanBd @alice-telescoop