[openstreetmap/openstreetmap-website] Fossgis OSRM server support (#2063)

[Tom Hughes]

tomhughes requested changes on this pull request.

This passed an OWG circular resolution so in principle we are good to go subject to technical review.

Other than the inline comments I have made I think my only outstanding question is how we want to refer to this - principally whether to include "OSRM" in the name or just refer to it as FOSSGIS?

Previously we have mixed both styles to some extent, although in some cases like Graphhopper the provider and the technology are the same, but in other cases (like MapQuest) we gave used the provider name and in others (like OSRM) we have used the technology name.

> @@ -2441,13 +2441,15 @@ en:
     directions:
       ascend: "Ascend"
       engines:
+        fossgis_osrm_bike: "Bicycle (FOSSGIS OSRM)"
+        fossgis_osrm_car: "Car (FOSSGIS OSRM))"
+        fossgis_osrm_foot: "Foot (FOSSGIS OSRM))"

We seem to have a superfluity of parentheses here...

> @@ -102,6 +102,7 @@ defaults: &defaults
   graphhopper_url: "https://graphhopper.com/api/1/route"
   mapquest_directions_url: "https://open.mapquestapi.com/directions/v2/route"
   osrm_url: "https://router.project-osrm.org/route/v1/driving/"

This line can be removed I think?

> @@ -119,9 +119,9 @@ function OSRMEngine() {
           namedRoad = false;
         }
 
-        if (step.maneuver.type.match(/^exit (rotary|roundabout)$/)) {
+        if (step.maneuver.type.match(/exit (rotary|roundabout)/)) {

What's the reason for removing the anchors from this, and the other matches? Is this server producing different directions that have something else before and after?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/2063#pullrequestreview-191351097
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20190110/4aec5871/attachment.html>