[openstreetmap/openstreetmap-website] Re add valhalla (PR #3848)

[Tom Hughes]

@tomhughes commented on this pull request.

My comments are all cosmetic, around making naming consistent.

I'm open to arguments that we shouldn't have the prefix, but we should be consistent in either having it or not having it - whether to have it really comes down to whether we're likely to have multiple instances of OSRM or Valhalla at any point...

> @@ -2927,6 +2927,9 @@ en:
         graphhopper_bicycle: "Bicycle (GraphHopper)"
         graphhopper_car: "Car (GraphHopper)"
         graphhopper_foot: "Foot (GraphHopper)"
+        valhalla_bicycle: "Bicycle (Valhalla)"
+        valhalla_car: "Car (Valhalla)"
+        valhalla_foot: "Foot (Valhalla)"

Can we make the prefix here `fossgis_valhalla` please, for consistency with other references to this engine?

>  // Doesn't yet support hints
 
-function FOSSGISEngine(id, vehicleType) {
+function OSRMEngine(id, vehicleType) {

Can we have this `FOSSGISOSRMEngine` please? and also rename the file to `fossgis_osrm.js`?

> @@ -0,0 +1,112 @@
+function ValhallaEngine(id, costing) {

Can we have this `FOSSGISValhallaEngine` please? and also rename the file to `fossgis_valhalla.js`?

> +            });
+          } else {
+            callback(true);
+          }
+        },
+        error: function () {
+          callback(true);
+        }
+      });
+    }
+  };
+}
+
+  OSM.Directions.addEngine(new ValhallaEngine("valhalla_car", "auto"), true);
+  OSM.Directions.addEngine(new ValhallaEngine("valhalla_bicycle", "bicycle"), true);
+  OSM.Directions.addEngine(new ValhallaEngine("valhalla_foot", "pedestrian"), true);

Can we add a `fossgis_` prefix to the engine names here please?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3848#pullrequestreview-1227885017
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/3848/review/1227885017 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20221222/48bc55c5/attachment.htm>