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

Add MaxSpeed Annotation to Directions API #772

Merged
merged 9 commits into from
Mar 30, 2018
Merged

Conversation

danesfeder
Copy link
Contributor

@danesfeder danesfeder self-assigned this Mar 26, 2018
*
* @since 2.1.0
*/
public static final String ANNOTATION_MAX_SPEED = "maxspeed";

Choose a reason for hiding this comment

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

A nitpicky observation... could this be ANNOTATION_MAXSPEED to match the spelling of maxspeed used everywhere else and to avoid confusion with a "max integer" type constant?

@danesfeder
Copy link
Contributor Author

@karenzshea good call, just updated. Also finished the MaxSpeed object if you want to 👀

@danesfeder danesfeder changed the title [WIP] Add MaxSpeed Annotation to Directions API Add MaxSpeed Annotation to Directions API Mar 27, 2018
/**
* String indicating the unit of speed, either as `kmh` or `mph`.
*
* @param unit either as `kmh` or `mph`

Choose a reason for hiding this comment

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

According to our fastidious colleagues, this technically should say km/h (because kmh apparently indicates a non-existent measure of kilometers of force blablabla 🙄 ). The api will return either km/h or mph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@karenzshea Yeah thanks for the clarification here. I was 🤔 this when looking at the api docs PR

@danesfeder danesfeder force-pushed the dan-add-maxspeed branch 2 times, most recently from 0af1024 to f213c4e Compare March 28, 2018 13:28
@karenzshea
Copy link

Wait - one last question! Does this need tests?

@danesfeder
Copy link
Contributor Author

@karenzshea definitely. added our model tests

@danesfeder
Copy link
Contributor Author

@osana can we sneak this in for 3.0 final release? 👀

@cammace cammace added this to the v3.0.0 milestone Mar 29, 2018
@cammace
Copy link
Contributor

cammace commented Mar 29, 2018

Added to 3.0 milestone

@danesfeder danesfeder merged commit ab23c5e into master Mar 30, 2018
@danesfeder danesfeder deleted the dan-add-maxspeed branch March 30, 2018 14:04
@osana osana mentioned this pull request Mar 30, 2018
14 tasks
@osana osana mentioned this pull request Apr 9, 2018
3 tasks
@osana
Copy link
Contributor

osana commented Apr 9, 2018

closed #740

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

Successfully merging this pull request may close these issues.

4 participants