Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Merging CartoDB changes back upstream #413

Closed
3 of 4 tasks
pnorman opened this issue Dec 1, 2015 · 6 comments
Closed
3 of 4 tasks

Merging CartoDB changes back upstream #413

pnorman opened this issue Dec 1, 2015 · 6 comments
Labels

Comments

@pnorman
Copy link
Contributor

pnorman commented Dec 1, 2015

I'm working at getting our carto changes merged back here to minimize different versions, and I'm wondering about what strategy to use to make it easy for you to merge.

In principle, the changes are

  1. 1. You can browserify carto and run it in the browser
  2. 2. carto will output to a JSON equivalent of Mapnik XML
  3. 3. A different mapnik-reference can be used. This is needed for passing to torque, which supports some additional properties and doesn't support some mapnik-specific ones
  4. 4. Supports frame-offset in selectors

The cartodb tree is https://github.com/CartoDB/carto/tree/master, but Github won't diff it since it was forked before mapbox/carto was made not a fork of less/less.css. This shell might help see changes while ignoring docs and compiled files: git diff mapbox/master cartodb/master -- git diff mapbox/master cartodb/master --name-only | grep -v '^dist/' | grep -v '.*.md$'``

My inclination is to first PR small non-browserify dependent changes, add browser support and tests, then add json output.

I'm not sure how to best handle 3 and 4. 3 is fairly easily to locally patch if we need to, but 4 is an addition that Mapnik wouldn't support.

@tmcw
Copy link
Contributor

tmcw commented Dec 4, 2015

We're happy to bring these changes back into the mainline - so far I'm a little confused by the direction of the browserification and note that we previously had another attempt at bringing changes back that made things browserify compatible, but not idiomatic or consistent with CommonJS - basic tenets like avoiding globals, using require(), and using a single entry point. If carto goes commonjs with browserify support, it should go all the way and follow the right patterns.

@pnorman
Copy link
Contributor Author

pnorman commented Dec 4, 2015

I saw the ping, but haven't managed to find you online when I've looked

@nebulon42
Copy link
Collaborator

nebulon42 commented Jan 7, 2017

As of 65fef93 I consider 1. done.

Is there any work still considered from the CartoDB side?

@pnorman
Copy link
Contributor Author

pnorman commented Jan 7, 2017

My work at CartoDB finished, so I'm not in a position to comment.

cc @rochoa

nebulon42 added a commit that referenced this issue Jun 4, 2017
…SON variant is used and then either output directly or afterwards converted to XML, ref #413
@nebulon42
Copy link
Collaborator

@rochoa Just to notify you that with f37bd50 we are now able to output a JSON variant of Mapnik XML. It still might need some polishing but basically works.

@nebulon42
Copy link
Collaborator

You can now specify your own reference and pass it to the renderer via the JS API. There is now also quite some documentation about that feature. With that I think carto is now flexible enough to support various use cases. Without interest from the CartoDB side I don't see No. 4 happening as I don't see the need for use cases outside of CartoDB.

I'd be happy if these additions are tested. As carto got a lot of bugfixing lately I think this would benefit CartoDB also. But I don't know how much the two forks have already diverged.

Closing for now.

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

No branches or pull requests

3 participants