One of my jobs as a maintainer of the OpenStreetMap Carto (osm-carto) stylesheet is to merge pull requests, which add new features, fix bugs, or otherwise change it.

The first step is to find a pull request by someone else that I can review. In this case I’m going to look at #1352, a simple pull request that changes the colour of text labeling kindergartens to be the same as text labeling schools.

The first thing I look at on a pull request which I’m looking to merge is the checkmark from Travis.

Travis CI checkmark If this isn’t there then it isn’t ready to merge. I can then check the detailed Travis status

Detailed Travis output

In order, this tells me

1. The MML file is valid JSON. This check isn’t so important now that the MML is generated from a YAML file, but it used to be very important when the JSON was edited directly.

2. When carto reads the MML file it generates valid Mapnik XML, which is 22906 lines long. By comparing with the build of master that the branch was taken from I can see that the line count has not changed. It’s possible to write Carto which will expand to hundreds of thousands of lines of XML, so this check is important.

   Because this pull request only changes a colour, if there was any change of the line count I would investigate it.

3. That the JSON MML file and the YAML file are in sync. Instead of an impossible to edit JSON file, osm-carto switched to a YAML file which is easier to read and perform code review on. This check is to make sure that the JSON file was correctly created by the coder.

Since everything checks out, I can go on to review the code. In this case, it’s a simple change that makes the colour agree with an earlier line so I can move right on to checking how it looks.

Being the first review of the day, I need to pull in any new changes. I start off by checking I don’t have any work in progress (or git stashing it), fetching any changes with git fetch --all, and updating my master with git checkout master && git merge upstream/master. Because I have all the regular osm-carto contributors git repositories set up as remotes, I can just check out the branch with git checkout math1985/kindergarten-color.

Like other maintainers, I use Kosmtik to do my stylesheet work, which I launch from another terminal window with node index.js serve --host 0.0.0.0 ~/osm/openstreetmap-carto/repo/project.yaml and then load Kosmtik in my browser. If trying to fix a rare bug I might need to load up a specific area, but normally I use the Geofabrik British Columbia extract, which only takes about four minutes to load.

Sometimes I know an area which has what I need to test, but in this case it’s easier to look for amenity=kindergarten on overpass-turbo, searching in an area I have loaded into my database.

I can then look to make sure that everything renders correctly and nothing is broken. If I’m happy with the reslts and I feel there’s been adequate discussion and review of the PR, I can merge it. I can either do this with Github, or in the case of conflicts from the command line. If I need to re-generate the JSON MML file when merging I can do this with git merge --no-commit, scripts/yaml2mml.py < project.yaml > project.mml, and git add project.mml