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

added toJson and fromJson to Directions models #854

Merged
merged 1 commit into from
Jul 24, 2018
Merged

Conversation

osana
Copy link
Contributor

@osana osana commented Jul 20, 2018

closes #854 , #696

@osana osana requested a review from tobrun July 20, 2018 15:55
@tobrun tobrun requested a review from danesfeder July 20, 2018 15:55
@osana osana force-pushed the osana-toFromJson branch 5 times, most recently from e396a9f to 0c63379 Compare July 20, 2018 21:24
Copy link
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

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

Just a minor typo and question about pending TODO - everything else looks 💯 thanks for running with this @osana 🚀

* This takes the currently defined values found inside this instance and converts it to a json
* string.
*
* @return a JSON string which represents this IntersectionLanes
Copy link
Contributor

Choose a reason for hiding this comment

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

Javadoc typo IntersectionLanes

.instruction("Turn left onto Adalbertstraße")
.build())
.intersections(intersections)
// TODO: Include the following
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this TODO need to be addressed before merging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danesfeder Thanks for flagging the TODOs.
I am not sure what kind of request would have those fields filled. Could you help me with those.

@osana osana force-pushed the osana-toFromJson branch from 0c63379 to c0b4f4e Compare July 23, 2018 22:54
Copy link
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

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

Thanks @osana 🚀

@osana osana merged commit 0dfbee7 into master Jul 24, 2018
@osana osana deleted the osana-toFromJson branch July 24, 2018 17:19
@danesfeder danesfeder mentioned this pull request Aug 6, 2018
13 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.

2 participants