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.

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

Fix custom environment variables logic #68

Merged
merged 3 commits into from
May 14, 2021
Merged

Conversation

rdewit
Copy link
Contributor

@rdewit rdewit commented May 13, 2021

In the current version ogr2ogr, custom environment variables get ignored due to a bug in the code.

This PR aims to fix this and adds relevant tests as well as updated documentation.

- RFC7946 does not appear to be a valid/documented env var
@wavded
Copy link
Owner

wavded commented May 13, 2021

Thanks for looking into this! Please fix the linting errors and should be good to go.

@rdewit
Copy link
Contributor Author

rdewit commented May 13, 2021

@wavded, thanks for the review. I fixed the styling issues by running npm run fmt and subsequently npm run fmt-check.

Note: on my Windows machine, running prettier actually changes all the files and a git diff shows something like the following for every file:

warning: LF will be replaced by CRLF in changelog.md.
The file will have its original line endings in your working directory

Adding the option "endOfLine": "auto" in .prettierrc will fix this but I'm not sure of unintended consequences.

Either way, fingers crossed that the checks will pass now.

@wavded wavded added the bug label May 14, 2021
@wavded wavded changed the title Bugfix: custom environment variables get ignored Fix custom environment variables logic May 14, 2021
@wavded wavded merged commit ab61d4b into wavded:master May 14, 2021
@wavded
Copy link
Owner

wavded commented May 14, 2021

Checks passed! Thx @rdewit for the contrib! Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants