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 getting mapmatching through POST method #948

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

osana
Copy link
Contributor

@osana osana commented Jan 8, 2019

closes #947

@osana osana added the WIP label Jan 8, 2019
@osana osana requested a review from Guardiola31337 January 8, 2019 18:20
@osana osana force-pushed the osana-map-maptching-post branch from 15dabc7 to 3cc596f Compare January 8, 2019 19:48
@osana osana removed the WIP label Jan 8, 2019
@osana osana force-pushed the osana-map-maptching-post branch from 3cc596f to d90e3ed Compare January 8, 2019 23:04
Copy link
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

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

I've left a couple of minor comments. Other than that this looks good to me 👍

Thanks @osana for adding the POST support to the Map Matching API.



/**
* Constructs the POSt call using the information passed in through the
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor thing - Typo *POSt instead of POST

*/
@FormUrlEncoded
@POST("matching/v5/{user}/{profile}")
Call<MapMatchingResponse> getPostCall(
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the get in

noting that it is a GET request? If so, should we name this method postCall instead?

@@ -338,6 +364,28 @@ public static Builder builder() {
private String[] waypointNames;
private String[] approaches;

/**
* Use GET method to request data (default).
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor comment - Shouldn't this be POST instead of GET? It seems it got reversed with the get method below 👇

@osana osana force-pushed the osana-map-maptching-post branch from daeb546 to 10c9a53 Compare February 4, 2019 19:03
@osana osana force-pushed the osana-map-maptching-post branch from 10c9a53 to 236b4e4 Compare February 4, 2019 20:32
Copy link
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

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

This is looking good 🚀

Thanks for addressing the feedback @osana

@osana osana merged commit bdb72fd into master Feb 4, 2019
@osana osana deleted the osana-map-maptching-post branch February 4, 2019 20:43
@osana osana mentioned this pull request Feb 12, 2019
14 tasks
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.

Add Map Matching API HTTP POST support
2 participants