-
Notifications
You must be signed in to change notification settings - Fork 818
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
Require Mapnik 3 and CartoCSS 0.16.0 #2473
Conversation
carto requires a |
I don’t understand well the mml/yaml/carto problem. Could you explain this further? |
Quick count: 5 of the 16 other PRs touch project.yaml: #2472 #2385 #2370 #2361 #2279. Are other affected? Changing the PRs would not be asking too much imho. Probably just a local rename of project.yaml to project.mml? |
#2423 would be probably also resolved by upgrading the minimum required version of CartoCSS with this PR. |
No problem with dropping the script, the PR was good for learning python some more. |
|
I left the extension at |
@@ -9,10 +9,8 @@ addons: | |||
node_js: | |||
- "0.10" | |||
install: | |||
- npm install carto@0.12.1 | |||
- npm install carto@0.16.0 |
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.
You should really use 0.16.3 as the versions before contain some regressions. For linting this does not make a difference but maybe is the wrong signal.
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.
This is in sync with with INSTALL.md
. I want to test with the minimum carto version and the latest, but that's a bigger change that I want to keep to a different PR.
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.
ok
Why? I think files recognized with their proper extensions would be the most natural solution (and .mml covering all as a fallback). |
In this case there is no proper extension. There is no convention that YAML files should be named .yml or .yaml. The current JSON is also not named .json. I don't know why Mapbox chose .mml and what it means. Maybe Map(box) markup language or something. The only change I see would be that Carto does no longer care about the extension at all. |
Fun fact - .mml comes from Cascadenik, a predecessor to CartoCSS created outside of Mapbox. In Cascadenik it contained XML. |
So, if MML doesn't mean anything special, let's make it The Right Way (TM). Three of these PRs are mine and I'm willing to fix them (except one which is probably going to waste anyway).
So was my Bash code fix, which I've just dropped. 😄 |
If we decide to merge this I'll probably just take the first commit, and redo the delete and rename instead of resolving merge conflicts. |
As far as I understand, we're positive about merging it. The only problem I see is release policy. We should make a bugfix release very soon and it would be v2.45.1 then, but if we merge also this code, this would be v3.0.0 probably, which would break our typical release time frame. Which solution do you think would be better? |
We could release 2.45.1 and 3.0.0 at the same time, and leave it to the server admins which version to install. |
Merging this code doesn't mean we need to immediately do a release |
It also enables #1126. |
Consensus seems to be we're ready to merge. I'll rebase later today. |
No, osm fr server do not run mapnik 3. |
So the milestone label for it was misleading, I've changed it now. |
Then I can merge it tomorrow (CET). |
This - drops Tilemill support; - increments the major version; and - removes some additional requirements for non-Latin languages
With CartoCSS 0.16.0 required, we don't need to create JSON anymore. This simplifies the contribution process for layer editing.
Small side-question: since I upgraded kosmtik and carto 0.16, my cache doesn't flush anymore, which is quite problematic when developing. Is this a known problem with the last version(s)? Is there a way to flush the cache manually? |
You mean |
My startup line is |
Thanks. Strange though, the cache used to be invalidated every time project files changed on disk. Removing |
Having started using kosmtik not too long ago i had the impression this was normal but now that you mention it it seems indeed rather odd. |
I started to clean the Kosmtik cache from some moment, so it probably worked before. We have a fresh ticket about this problem now: |
I'm in same situation as imagico. |
I'm pretty sure that this is unrelated to carto 0.16.x. |
Just wondering whether there is a reason why the latest YAML format of project.mml has the same basename of the Tilemill JSON format. AFAIK Tilemill for Windows does not allow to change the project filename. Even if Tilemill is no more supported now, one can still try to keep on using it and in case there is the need to rename project.mml to project.yaml, restore yaml2mml.py (or equivalent command) and generate project.mml in JSON format: this might lead to errors/confusion with the original YAML file. Wouldn’t be more appropriate naming the new project.mml (YAML) to osm-carto.mml or similar? |
If someone is using tilemill I believe they need to be using a version from source to get an appropriate Mapnik version, which should also have a CartoCSS version which supports reading YAML. They'll have problems editing the layers through Tilemill, but given Tilemill's interface, there's problems doing any editing through it. |
I haven't checked the Ubuntu version yet. I am afraid that with the Windows version (which is the only native way to run openstreetmap-carto with Windows and without an Ubuntu VM - just run, not edit) there is no possibility to perform a manual update (and manually updating node_modules\carto does not provide a working product). |
My point is that you can't use the prebuilt tilemill binaries, since we require Mapnik 3, and the version of Mapnik that came with v0.10.1 is from 2012, so we don't need to worry if something else is broken on Tilemill v0.10.1 |
Rename project.yaml to project.mml following gravitystorm#2473
Rename project.yaml to project.mml following gravitystorm#2473
This should wait until after v2.45.0.
Fixes #2458, #2198.
Enables #2470, #2045