-
Notifications
You must be signed in to change notification settings - Fork 160
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
Update LedgerResponse
and AccountResponse
, remove outdated fields, and add missing fields.
#549
Update LedgerResponse
and AccountResponse
, remove outdated fields, and add missing fields.
#549
Conversation
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.
for consideration, add reference to https://developers.stellar.org/docs/fundamentals-and-concepts/stellar-data-structures/accounts to the AccountResponse
class docs, noticed you added similar link for LedgerResponse
, that content looks great!
Also, do you feel it's worthwhile to refactor AccountResponse
to use lombok also, noticed you included that refactoring on pr for LedgerResponse
, understand if not, as it's not related to the change scope.
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.
or consideration, add reference to https://developers.stellar.org/docs/fundamentals-and-concepts/stellar-data-structures/accounts to the AccountResponse class docs, noticed you added similar link for LedgerResponse , that content looks great!
Added.
Also, do you feel it's worthwhile to refactor AccountResponse to use lombok also, noticed you included that refactoring on pr for LedgerResponse, understand if not, as it's not related to the change scope.
I did not refactor AccountResponse using Lombok because it would cause breaking changes. I think I will have a separate PR later to use Lombok to rewrite all the code.
|
||
@SerializedName("_links") | ||
private final Links links; |
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.
awesome!
Closes #287, Closes #455