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
Makefile: Replace static DB properties with the variables #1222
Conversation
Results evaluating commit f67ce37 (merged with base 5e37a81 as 5b2c8ca). See run details. PostgreSQL DB size in MB: 2792 ⇒ 2791 (-0.0% change)
expand for details...
|
I'm not too sure this will work -- the updated forgot -- we also allow old style env vars for compatibility, so it would be more complex -- P.S. Considering that we want to get rid of the tm2source/mapnik altogether, i'm not even sure its worth even dealing with |
The |
good point about the .env file, i did not realize it was included (even though I was the one who added it there :) ). I disagree about vars not defined -- anyone who uses their own .env file could still have those settings. So either we should remove their support everywhere in the tools, and officially declare them as unsupported, or we should consistently support them everywhere. |
I added a check about the old var styles. If one of the old variable is existing, then it is used else the new one. Same as in the tools scripts: Solution in the Makefile:
I think it is valid to support both variable names but I think also that after 2 years it is legit to remove the old var styles. Maybe for a major relase in future. A simple test which can added to the Makefile to see which value is taken:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me! I agree that in the next major we should probably remove support for the legacy PG vars
No description provided.