-
Notifications
You must be signed in to change notification settings - Fork 69
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
Support for Flight Order #51
Conversation
README.md
Outdated
@@ -238,6 +238,10 @@ FlightOfferSearch[] flightOffersSearches = amadeus.shopping.flightOffersSearch.g | |||
// body can be a String version of your JSON or a JsonObject | |||
FlightOfferSearch[] flightOffersSearches = amadeus.shopping.flightOffersSearch.post(body); | |||
|
|||
// Flight Order Management | |||
// The flightOrderID comes from the Flight Create Orders (in test environment it's temporary) | |||
com.amadeus.resources.FlightOrder order = amadeus.booking.flightOrder("eJzTd9f3NjIJdzUGAAp%2fAiY=").get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we avoid using fully-qualified name of the class, like in the rest of the examples?
The argument to amadeus.booking.flightOrder() is a flightOrderID - this fact is not very obvious.
* | ||
* <pre> | ||
* com.amadeus.resources.FlightOrder order = amadeus.booking.flightOrder.( | ||
* "eJzTd9f3NjIJdzUGAAp%2fAiY=").get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mention that "eJzTd9f3NjIJdzUGAAp%2fAiY=" is an example?
} | ||
|
||
@ToString | ||
public class Traveler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no existing class for "Traveler" entity?
|
||
@ToString | ||
public class Name { | ||
protected Name() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same - Is there a specific reason to override the constructor?
@ToString | ||
public class Phone { | ||
|
||
protected Phone() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same - Is there a specific reason to override the constructor?
|
||
@ToString | ||
public class Document { | ||
protected Document() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same - Is there a specific reason to override the constructor?
|
||
@ToString | ||
public class Contact { | ||
protected Contact() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same - Is there a specific reason to override the constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From our discussions, there are few updates that we skipped to maintain consistency of the rest of the SDK code. There should be a separate issue to fix these architectural decisions in the future.
No description provided.