Save Atlas to ProtobufAtlas similar to TextAtlas #99

lucaspcram · 2018-03-21T23:53:09Z

Since I changed the build flow of the Atlas project to accommodate protobuf, I think it may be a good idea to create a protobuf (or some other name) branch on the repo. I don't want to break peoples' Eclipse workflow on the dev branch until we are further along with this update. In the interest of this objective, we should probably close this PR and I'll reopen it on the protobuf branch once that is created.
EDIT: After discussion with Matthieu, we have decided to not create a protobuf branch on the repo. Instead, I will push to a protobuf branch on my fork, then submit PRs against dev for review. We can discuss, I'll make changes in my fork, then resubmit or restart the PR. When the feature is complete, we will consolidate and pull the changes into dev.

Other than that, I was hoping we could have an initial discussion about the proposed changes, and make sure I am moving in the right direction before I really start cranking out more code.

…md for instructions on how to get the project working with Eclipse.

MikeGost

You've officially become our Proto expert! Really nice and clean code. Some minor suggestions inline.

MikeGost · 2018-03-22T00:00:19Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        return protoTags;
+    }
+
+    private ProtoLocation convertLocationToProtoLocation(final Location location)


Check out the Converter and TwoWayConverter interfaces, it might be a good pattern to follow here as well. Create a proto conversion package that has all the converters (with convert and backwardConvert), then use them here in the builder.

Sounds good! James actually mentioned the exact same thing.

MikeGost · 2018-03-22T00:04:29Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+            final ProtoTag.Builder tagBuilder = ProtoTag.newBuilder();
+
+            // TODO what happens if entry.getValue() returns null?
+            // this could happen if someone inserted a null entry at that key


I'd say in the case of a null key or value, throw away the entry and log a warning.

MikeGost · 2018-03-22T00:14:53Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+            }
+            return result;
+        }
+        // TODO do something smarter here?


I think this is fine, I would add the error to the re-thrown exception so it doesn't get lost. Possibly the tagsList as well.

throw new CoreException("Unable to parse proto tags.", error);

MikeGost · 2018-03-22T00:15:22Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        {
+            protoAtlasContainer = ProtoAtlasContainer.parseFrom(resource.readBytesAndClose());
+        }
+        // TODO do something smarter here?


Only add the caught exception to the CoreException argument list so the stack trace isn't lost.

throw new CoreException("Error parsing the protobuf", exception);

MikeGost · 2018-03-22T00:19:39Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+            protoLineBuilder.setId(line.getIdentifier());
+
+            final List<ProtoLocation> protoLocations = StreamSupport
+                    .stream(line.getRawGeometry().spliterator(), false)


An alternative way (not sure if it's cleaner) would be line.asPolyLine().stream().map(...).collect(...);

I'll change this. For some reason I couldn't get that to work initially so went with the StreamSupport solution. I was probably just spelling something wrong.

MikeGost · 2018-03-22T00:23:19Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        final long numberOfRelations = 0;
+        final AtlasSize atlasSize = new AtlasSize(numberOfEdges, numberOfNodes, numberOfAreas,
+                numberOfLines, numberOfPoints, numberOfRelations);
+        final AtlasMetaData atlasMetaData = new AtlasMetaData(atlasSize, true, "unknown",


Wondering if we should have any kind of tag to specify this atlas was built from Proto?

I'll take a look at that class and see if there's something I can add.

Oh I just mean add an entry into the Metadata tags saying source=proto and proto_version=xx (not sure if you can read that dynamically, but assuming yes).

Ah ok yeah can do. Did you have some fancy method in mind or should I just write code that greps build.gradle?

I thought there might be an easier way! If there's not, I wouldn't worry about it.

I'll mark it as TODO and if some simple way presents itself I'll be sure to implement it.

lucaspcram · 2018-03-22T13:47:08Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        resource.writeAndClose(protoAtlas.toByteArray());
+    }
+
+    private List<ProtoTag> buildProtoTagListFromTagMap(final Map<String, String> tags)


Perhaps this function can also go in the Converter package?

Converter back and forth between List and Map<String,String>? That sounds like it would work.

lucaspcram · 2018-03-22T13:47:32Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        });
+    }
+
+    private Map<String, String> parseTags(final List<ProtoTag> tagsList)


Perhaps this class can go in the Converter package?

matthieun

Very good looking code and protos! I have a few questions/requests regarding the gradle/eclipse integration.

matthieun · 2018-03-22T16:58:54Z

ECLIPSE_README.md

+```
+
+4. Back in Eclipse, right click the Atlas project in Package Explorer and click "Refresh"
+  1. NOTE: You should now see a "gen" folder appear in the package explorer


So if you do right click > Gradle > Refresh and the build.gradle defines gen as a source folder, it should be picked up by Eclipse automatically. There should not be a need to go in the "cofigure build path" menu. Have you tried that?

The problem I had with that is for some reason it just goes ahead and deletes the generated code folder, and I can't figure out why. It may have something to do with the source set config. I am currently playing around with that on a new branch on my local machine.

matthieun · 2018-03-22T17:01:12Z

build.gradle

+    {
+        java
+        {
+            srcDir file('gen/main/java')


How about calling that folder src/generated/java? It would fall nicely with the other src/main/java, src/test/java and src/integrationTest/java...

See note regarding ECLIPSE_README above

matthieun · 2018-03-22T17:02:01Z

build.gradle

+    generatedFilesBaseDir = 'gen/' 
+
+    protoc {
+        artifact = 'com.google.protobuf:protoc:2.6.1'


I think it would make sense to use the version from dependencies.gradle here for consistency.

Currently investigating the proper syntax for this.

matthieun · 2018-03-22T17:03:05Z

config/checkstyle/suppressions.xml

@@ -9,5 +9,6 @@
 	<suppress checks="MultipleStringLiterals" files="src/test/*" />
 	<suppress checks="MultipleStringLiterals" files="src/integrationTest/*" />
 	<suppress checks="InnerAssignment" files="src/test/*" />
-	<suppress checks="InnerAssignment" files="src/integrationTest/*" />
+    <suppress checks="InnerAssignment" files="src/integrationTest/*" />


Looks like indentation is not aligned here!

Ah that's because suppressions.xml is indented with tabs. I have Eclipse set to expand tabs to spaces, which I forgot to disable while editing that file. I'll go ahead and fix that. Perhaps it would be better to change suppressions.xml to be indented with spaces? I could update that as well if you think it's a good idea.

I will change the indentation to spaces.

matthieun · 2018-03-22T17:03:57Z

dependencies.gradle

@@ -56,5 +57,6 @@ project.ext.packages = [
            core: "com.fasterxml.jackson.core:jackson-core:${versions.jackson_core}",
            databind: "com.fasterxml.jackson.core:jackson-databind:${versions.jackson_databind}"
    ],
+    protobuf_java: "com.google.protobuf:protobuf-java:2.6.1"


Here use ${versions.protobuf_java}

matthieun · 2018-03-22T17:05:20Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        }
+        catch (final InvalidProtocolBufferException exception)
+        {
+            throw new CoreException("Error deserializing the ProtoAtlasContainer", exception);


Here I would pass the name of the resource as well!

throw new CoreException("Error deserializing the ProtoAtlasContainer from {}", resource.getName(), exception);

Awesome I'll add that in.

matthieun · 2018-03-22T17:09:00Z

build.gradle

+        recompileProto.dependsOn(deleteGeneratedProto)
+        recompileProto.dependsOn(generateProto)
+
+        compileJava.dependsOn(generateProto)


Could you figure out what gradle tasks Eclipse's gradle plugin calls? Once we know that we could just add someTask.dependeOn(recompileProto)?

I will look into it. Some cursory googling I did the other day turned up nothing. I may have to dive deep into documentation.

matthieun · 2018-03-22T17:12:02Z

src/main/java/org/openstreetmap/atlas/utilities/conversion/ProtoTagListConverter.java

+
+            if (entry.getKey() == null)
+            {
+                logger.warn("buildProtoTagListFromTagMap found null key");


Not sure if it is possible, but adding more context to those logs might help...

It's definitely possible. I'll change that in the next couple of commits.

jwpgage

Really nice @lucaspcram! Easy to follow and well commented. Couple comments about potential places to reuse code.

jwpgage · 2018-03-22T17:38:48Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+            final Map<String, String> tags = protoTagListConverter.convert(protoLine.getTagsList());
+            builder.addLine(identifier, geometry, tags);
+        });
+    }


parseAreas and parseLines seem to be identical apart from different parameters. Would it be worth consolidating these two and passing a generic proto object?

This is something Mike and I discussed in our 1on1. We decided for now to have them separate purely so that the design follows the API in a logically consistent way, in spite of the code reuse. Later on if it seems to be an issue we can pretty easily refactor it away.

jwpgage · 2018-03-22T17:40:16Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+            protoAtlasBuilder.addLines(protoLineBuilder.build());
+        }
+        protoAtlasBuilder.setNumberOfLines(numberOfLines);
+    }


Same comment as above

jwpgage · 2018-03-22T17:41:00Z

src/main/java/org/openstreetmap/atlas/utilities/conversion/ProtoLocationConverter.java

+        final Latitude latitude = Latitude.dm7(protoLocation.getLatitude());
+        return new Location(latitude, longitude);
+    }
+}


These are perfect for the utilities.conversion package, nice!

MikeGost · 2018-03-22T17:58:20Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+                    .map(protoLocationConverter::convert).collect(Collectors.toList());
+            final PolyLine geometry = new PolyLine(shapePoints);
+            final Map<String, String> tags = protoTagListConverter.convert(protoArea.getTagsList());
+            builder.addLine(identifier, geometry, tags);


Should this be addArea?

Also, just a heads up: The PolyLine class stores all points. Since areas are closed lines, the first and last point is identical. The Polygon class expects the last point to not be the first point, that is implicit. So when creating the Polygon, make sure the last point is not equal to the first point, otherwise it will get duplicated.

Yes. Some classic cut and paste shenanigans right there.

My unit test is actually failing due to this exact reason. The Atlas that gets written to the file has an Area with a Polygon, which repeats the start and end node. The Atlas that is read back in has a Linestring without the repeated point.

…n issues.

MikeGost · 2018-03-22T22:21:54Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+            numberOfAreas++;
+            protoAtlasBuilder.addAreas(protoAreaBuilder.build());
+        }
+        protoAtlasBuilder.setNumberOfLines(numberOfAreas);


setNumberOfLines --> setNumberOfAreas

Unbelievable. This is embarrassing.

MikeGost · 2018-03-22T22:24:11Z

src/main/java/org/openstreetmap/atlas/utilities/conversion/ProtoLocationConverter.java

+ *
+ * @author lcram
+ */
+public class ProtoLocationConverter implements TwoWayConverter<ProtoLocation, Location>


Nice-To-Have: Corresponding unit test for each converter. You're doing this in your Builder Test, but if someone needs to change this, would be good to have a unit test for each. If you feel this is too much, feel free to disregard.

This is actually a good idea. Shouldn't be too hard. It'll be in a future commit.

NOTE: Would it be better to move this class to org.openstreetmap.atlas.geography.converters?

Yep - see my original comment about a new package for these! I'd suggest org.openstreetmap.atlas.geography.converters.proto

jklamer

👍

jklamer · 2018-03-23T17:25:31Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        parsePoints(builder, protoAtlasContainer.getPointsList());
+        parseLines(builder, protoAtlasContainer.getLinesList());
+        parseAreas(builder, protoAtlasContainer.getAreasList());
+


Are there no relations being added to the new packed Atlas or am I missing something?

We're getting there. It's still in development :)

jklamer · 2018-03-23T17:28:56Z

src/main/java/org/openstreetmap/atlas/utilities/conversion/ProtoTagListConverter.java

+ *
+ * @author lcram
+ */
+public class ProtoTagListConverter implements TwoWayConverter<List<ProtoTag>, Map<String, String>>


I dig all the converters

I think I doubled the number of converters we have hahaha

See org.openstreetmap.atlas.geography.converters and org.openstreetmap.atlas.geography.converters.jts for some converter love :)

lucaspcram · 2018-03-23T18:51:07Z

It builds locally I swear.

lucaspcram · 2018-03-27T21:43:25Z

Thanks for the feedback guys!

@jwpgage These are good points. It might be a good idea to set up an email filter for my PRs, that way your inbox doesn't get spammed.

@MikeGost

Currently working on this. It turns out the Google gradle-protobuf plugin hardcodes your generated source set to be 'main/java', so it's not possible to have a 'src/generated/java' folder like @matthieun wanted. See this discussion on the gradle-protobuf plugin issue page.
Awesome sounds good! I think the process was really productive. If we can deal with the increased email traffic, I think it would be cool to have more interactive PRs like this on the repo. It really encourages testing and sanity checking.

no longer forces user to manually update the build path

MikeGost · 2018-03-29T17:19:32Z

ECLIPSE_README.md

+3. Once you are done adjusting the .proto files, open a command line and run
+```
+$ cd /path/to/local/Atlas/repo
+$ ./gradlew clean build -x test -x integrationTest


Do we need to update these instructions with the latest gradle task?

I am going to be removing this file and updating the regular README!

MikeGost · 2018-03-29T17:20:36Z

build.gradle

+
+        task deleteGeneratedProto {
+            // the Google gradle-protobuf plugin automatically registers this task
+            // to be run as a pre-requisite to 'clean'. Since Eclipse's "Refresh Gradle Project"


Potential rabbit hole: have you tried this on IntelliJ?

No. And it's something I know needs to be done.

One option is to create a github issue for it and address it after this is merged.

MikeGost · 2018-03-29T17:22:45Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+                    .map(protoLocationConverter::convert).collect(Collectors.toList());
+            final Polygon geometry = new Polygon(shapePoints);
+            final Map<String, String> tags = protoTagListConverter.convert(protoArea.getTagsList());
+            // TODO see Mike's PR note about duplicate points, since start and end are the same for


Can this TODO be removed?

MikeGost · 2018-03-29T17:23:54Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+
+    private void parseAreas(final PackedAtlasBuilder builder, final List<ProtoArea> areas)
+    {
+        final ProtoLocationConverter protoLocationConverter = new ProtoLocationConverter();


We can rely on static converters instead of initializing them for each parsing function. This should trim the code a little to avoid initializing a new converter each time.

I'll change that!

MikeGost · 2018-03-29T17:25:34Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        });
+    }
+
+    private void parseLines(final PackedAtlasBuilder builder, final List<ProtoLine> lines)


Did you want to combine parseLines and parseAreas? The only difference is creating a PolyLine vs. a Polygon, which can be handled with a boolean flag.

I'm going to take a look at some of these things today. Please don't merge yet!!

MikeGost · 2018-03-29T17:26:11Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+        });
+    }
+
+    private void parsePoints(final PackedAtlasBuilder builder, final List<ProtoPoint> points)


Similar comment on combining parseNodes and parsePoints

MikeGost · 2018-03-29T18:53:20Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+            final RelationBean bean = parseRelationBean(protoRelation);
+            final Map<String, String> tags = protoTagListConverter
+                    .convert(protoRelation.getTagsList());
+            // TODO should identifier and OSMIdentifier be the same?


They shouldn't be the same, one is the Atlas identifier, and one is the OSM identifier. Atlas identifier is padded with the country-slicing and way-sectioning digits. We should make a design decision on which one is passed in. This is somewhat tricky - since atlas identifier must be unique per type of entity (a Relation and an Edge can have the same atlas identifier, but no two Relations can have the same atlas identifier). So if this is a true representation of an Atlas, they should pass in the Atlas Identifier and we can leverage our existing factories to figure out the OSM Identifier based on the atlas identifier (remove the 6 digit padding). Let me know if that made sense!

Hmm I see. I guess my question is, what should I be passing in for that second parameter? Will I need to have some additional code right here in this function to calculate the correct OSM id? Is there a reason the other builder 'add' methods for Node, Edge, Point, Line, etc. do not have an OSM id parameter?

Check out ReverseIdentifierFactory - it can return the OSM identifier given an Atlas identifier. There is currently an issue for the other "add" methods to be consistent with this one. Sent you a pointer offline.

MikeGost · 2018-03-29T18:55:29Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+                final ItemType type = ItemType.forEntity(member.getEntity());
+                beanBuilder.setMemberType(ProtoRelation.ProtoItemType.valueOf(type.getValue()));
+
+                // TODO there may be a problem here if there are more relations in this atlas than


Doubt we will have 2 billion relations in an atlas. I would be more concerned with number of edges (due to way-sectioning and bi-directionality)

Hmm. This would be a problem as well, since the ProtoAtlasContainer, when deserialized into an object, stores each of the feature types in an ArrayList - this is built into protobuf. So in memory you have a List of Nodes, Points, Lines, Edges, etc. I would imagine you would never have 2 billion features in one of those categories in a single atlas.

Let's hope not, but theoretically this could happen! This is worth noting and identifying as a limitation of the current proto implementation.

MikeGost · 2018-03-29T18:56:09Z

src/main/java/org/openstreetmap/atlas/geography/atlas/command/PackedToProtoAtlasSubCommand.java

+ *
+ * @author lcram
+ */
+public class PackedToProtoAtlasSubCommand implements FlexibleSubCommand


Very clean! Nicely done.

MikeGost

Nicely done! Couple of nits, addressing TODOs, and minor code duplication removal possibilities.

lucaspcram · 2018-03-29T19:34:46Z

Hey @MikeGost ! Thanks, I'll be taking care of those in a coming commit. There are actually still some lingering gradle issues, and at this point I'm uncertain as to the right solution. Definitely will need to work with @matthieun to get this resolved.

… directory.

MikeGost

Nicely done! Just added a bunch of nits.

MikeGost · 2018-03-29T22:23:32Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+ */
+public class ProtoAtlasBuilder
+{
+    private final ProtoLocationConverter protoLocationConverter = new ProtoLocationConverter();


One nit: Why not make all the converters private static final variables?

Haha. I had this initially, but it looked more verbose with the all-caps and the fact that the style checker prepends ProtoAtlasBuilder. to the front of every variable reference. I can change it back to static though if that is preferred.

MikeGost · 2018-03-29T22:28:02Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+{
+    private final ProtoLocationConverter protoLocationConverter = new ProtoLocationConverter();
+    private final ProtoTagListConverter protoTagListConverter = new ProtoTagListConverter();
+    private final ReverseIdentifierFactory reverseIDFactory = new ReverseIdentifierFactory();


Nit: To avoid abbreviations: reverseIdentifierFactory

MikeGost · 2018-03-29T22:28:49Z

src/main/java/org/openstreetmap/atlas/geography/atlas/builder/proto/ProtoAtlasBuilder.java

+                final ItemType type = ItemType.forEntity(member.getEntity());
+                beanBuilder.setMemberType(ProtoRelation.ProtoItemType.valueOf(type.getValue()));
+                protoRelationBuilder.addBeans(beanBuilder.build());
+


Nit: extra line

matthieun · 2018-03-29T23:22:22Z

I will leave the honor to @MikeGost to merge that massive (in terms of commits) PR :)

Merge public changes

lucaspcram added 6 commits March 19, 2018 13:22

Fully integrated protobuf build flow with gradle. See ECLIPSE_README.…

bb7706a

…md for instructions on how to get the project working with Eclipse.

Fixed spotless application to ignore generated source code.

1890bc8

Initial, naive implementation of proto specs for Tags and Points.

a03cade

Small updates to the README files.

4e05e34

Started implementation of naive ProtoAtlasBuilder

18611a3

Implemented Point and Line packing.

f382466

MikeGost reviewed Mar 22, 2018

View reviewed changes

lucaspcram commented Mar 22, 2018

View reviewed changes

matthieun added the New Feature label Mar 22, 2018

Stylistic and structural changes per PR notes.

fd9af27

matthieun reviewed Mar 22, 2018

View reviewed changes

matthieun changed the title ~~Initial Protobuf PR - need a public protobuf branch~~ Do Not Merge: Save Atlas to ProtobufAtlas similar to TextAtlas Mar 22, 2018

Trying to get Areas working with the serializer.

fbdaacc

jwpgage reviewed Mar 22, 2018

View reviewed changes

MikeGost reviewed Mar 22, 2018

View reviewed changes

lucaspcram added 6 commits March 22, 2018 11:16

Fixed tab indentation to use spaces in suppressions.xml

c9720fe

Trying to fix the Area serialization. Currently failing due to Polygo…

bc15496

…n issues.

Fixed failing tests, which were due to bad copy paste.

a804b72

Integrated a few PR style suggestions.

40437f7

Added an area count to the metadata.

70abe50

Last of the copy paste shenanigans that I messed up. I hope. lol

1eba0c9

MikeGost reviewed Mar 22, 2018

View reviewed changes

lucaspcram added 3 commits March 22, 2018 16:01

Ok. If this is not the last copy-paste bug then I resign.

d08f1a8

Initial drafts of Node and Edge protos.

5aa04a8

Added unit tests for the proto converter classes.

539337e

jklamer reviewed Mar 23, 2018

View reviewed changes

Changed packages on the converter classes per PR suggestions.

0bfec87

Forgot to stage this file.

5ce39d5

lucaspcram added 4 commits March 28, 2018 14:08

Tweak to buildflow. Gradle project refresh still requires a rebuild, but

0e19a0b

no longer forces user to manually update the build path

Small tweak to prevent Gradle Refresh from deleting the generated code.

2678b77

Added a task to actually clear the generated code.

bc6f404

Slight tweak to build.gradle to fix beforeMerged

fe1e9a1

matthieun approved these changes Mar 29, 2018

View reviewed changes

MikeGost reviewed Mar 29, 2018

View reviewed changes

Hopefully ready for merge

3918bb8

MikeGost reviewed Mar 29, 2018

View reviewed changes

MikeGost approved these changes Mar 29, 2018

View reviewed changes

lucaspcram added 4 commits March 29, 2018 13:35

Finalized fixes to gradle. Everything should work now.

ee43801

Slight change to README

eeeb127

Fixed checkstyle and spotless suppressions to deal with new generated…

0d4e3f0

… directory.

Final changes before merge. Hopefully we are ready to go.

9657bac

MikeGost approved these changes Mar 29, 2018

View reviewed changes

Fixed some nits.

79e7450

matthieun changed the title ~~Do Not Merge: Save Atlas to ProtobufAtlas similar to TextAtlas~~ Save Atlas to ProtobufAtlas similar to TextAtlas Mar 29, 2018

MikeGost merged commit 03fa090 into osmlab:dev Mar 29, 2018

lucaspcram deleted the protobuf branch March 30, 2018 16:05

matthieun added this to the 5.0.11 milestone Apr 11, 2018

gitclonefun pushed a commit to gitclonefun/atlas that referenced this pull request Feb 14, 2019

Merge pull request osmlab#99 from maps-osm/dev-merge

361f500

Merge public changes

Save Atlas to ProtobufAtlas similar to TextAtlas #99

Save Atlas to ProtobufAtlas similar to TextAtlas #99

Conversation

lucaspcram commented Mar 21, 2018 • edited by matthieun Loading

MikeGost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthieun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwpgage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklamer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucaspcram commented Mar 23, 2018

lucaspcram commented Mar 27, 2018

MikeGost Mar 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeGost Mar 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucaspcram commented Mar 21, 2018 •

edited by matthieun

Loading

MikeGost Mar 29, 2018 •

edited

Loading

MikeGost Mar 29, 2018 •

edited

Loading