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

Ascendex Apis #4531

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Ascendex Apis #4531

wants to merge 9 commits into from

Conversation

wodepig
Copy link

@wodepig wodepig commented Jul 20, 2022

  • Add most of the REST APIs
  • rewrite Dtos of lombok
  • AscendexDigest.java:Fixed url path problem of authentication signature
  • ascendexApiInfo.json:The API information needed for the real test
  • add param

85365 added 9 commits July 14, 2022 21:10
/v1/{accountCategory}/products  api
v2/assets api
v1/spot/ticker api
update:
Improving test classes
Balance apis
wallet apis
fix :
AscendexDigest
update:
enums group
add simple AscendexPlaceOrderRequestPayload
private String userUID;
private Boolean tradePermission;
private Boolean transferPermission;
private Boolean viewPermission;
Copy link
Collaborator

Choose a reason for hiding this comment

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

please fix you coding conventions, we do not use tabs, indent should be 2 spaces

this also indicates that you did not build the project using maven as it would fix formatting for you

*/
@Data
@NoArgsConstructor
@AllArgsConstructor
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use @Value instead of the above

/**
* Package:org.knowm.xchange.ascendex.dto.account
* Description:
*
Copy link
Collaborator

Choose a reason for hiding this comment

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

these comments look unnecessary

AccountCategory toAccount;

public void setAsset(String asset) {
this.asset =asset==null?null: asset.toUpperCase();
Copy link
Collaborator

Choose a reason for hiding this comment

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

mapping should be done in *Adapter classes, not in dtos

dtos should represent what came from the API

@walec51
Copy link
Collaborator

walec51 commented Jul 30, 2023

is there any progress on this PR?

if not I will have to close it

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