Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSRM dependency injection #75

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

morganherlocker
Copy link
Contributor

Overview

This PR updates sharedstreets-js to make osrm an optional install. This is helpful for deploying to environments with limited resources (raspberry pi, lambda) or difficult compilation targets (redhat, windows) where the functionality that OSRM provides is not needed.

Changelog

  • osrm moved to dev dependency
  • osrm is now installed and instantiated by the user
  • graph constructor now accepts osrm instance
  • tests updated to reflect new interface

?s

@kpwebb this feels almost there, but I wasn't sure how to best instantiate the osrm instance in tests. My guess is that we want to download the cache in the envelope and have the user perform the build step on their own, but want to get your thoughts here.

cc @emilyeros

@morganherlocker morganherlocker added behavior dependencies Pull requests that update a dependency file labels Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants