Skip to content

feat: add optional language parameter to Valhalla provider#905

Open
poojithacsef wants to merge 2 commits into
stadiamaps:mainfrom
poojithacsef:main
Open

feat: add optional language parameter to Valhalla provider#905
poojithacsef wants to merge 2 commits into
stadiamaps:mainfrom
poojithacsef:main

Conversation

@poojithacsef
Copy link
Copy Markdown

"Adds an optional language field to the Valhalla request payload."

Copy link
Copy Markdown
Contributor

@ianthetechie ianthetechie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that a better direction for this would be an options builder which makes the details more discoverable. Unfortunately this is probably quite a lot of work (there are hundreds of options), so we'll probably want to always keep a way to add arbitrary keys like we have today.

I'm guessing you want this more immediately for use in your app, and the good news is that you can already pass options via an arbitrary map :) See the docstring a few lines above for an example. At the top level you can just pass {"language": "de"} etc.... a new struct field isn't necessary.

Removed the language field from the Valhalla struct and updated related methods to use options for language settings.
@poojithacsef
Copy link
Copy Markdown
Author

I've updated the PR to remove the top-level struct field. Instead, I added a chainable with_language convenience method that injects the key straight into the existing options map behind the scenes. This keeps the architecture clean while making the feature easily discoverable. Let me know if this looks good to merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants