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.

Sign up for GitHub

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

Jump to bottom

AND-5815 Add AptosApi #435

Merged
merged 3 commits into from
Jan 16, 2024
Merged

AND-5815 Add AptosApi #435

merged 3 commits into from
Jan 16, 2024

Conversation

Mama1emon
Copy link
Contributor

@Mama1emon Mama1emon commented Jan 16, 2024 •
edited
Loading

  1. Add AptosApi and request/response models
  2. Fix detekt warnings from develop

@Mama1emon Mama1emon force-pushed the feature/AND-5815_Add_AptosApi branch from e3ffe1d to 046a376 Compare January 16, 2024 08:25
@Mama1emon Mama1emon force-pushed the feature/AND-5815_Add_AptosApi branch from b3a3a4c to cfe93da Compare January 16, 2024 08:37
Sateetas
Sateetas reviewed Jan 16, 2024
View reviewed changes
blockchain/build.gradle Outdated
@@ -55,6 +55,7 @@ dependencies {
implementation 'com.squareup.retrofit2:converter-moshi:2.6.0'
implementation 'com.squareup.moshi:moshi:1.13.0'
implementation "com.squareup.moshi:moshi-kotlin:1.13.0"
implementation 'com.squareup.moshi:moshi-adapters:1.9.1'
Copy link
Contributor

Choose a reason for hiding this comment

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

есть смысл такую же версию поставить, как и у остальных компонентов

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Готово

Sateetas
Sateetas reviewed Jan 16, 2024
View reviewed changes
...hain/src/main/java/com/tangem/blockchain/blockchains/aptos/network/response/AptosResource.kt Outdated

@JsonClass(generateAdapter = true)
data class CoinResource(
@Json(name = "data") val coin: CoinData,
Copy link
Contributor

Choose a reason for hiding this comment

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

data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ключевые слова котлина нельзя использовать

Copy link
Contributor

Choose a reason for hiding this comment

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

точно!
я к тому, чтобы при использовании не было coin.coin, потому что внутренняя сущность называется тоже coin, мб подумать чтобы как-то переименовать внешнюю?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Хорошее замечание.
Что думаешь насчет coinData?
Разрабы блокчейна так спроектировали JSON, что очень трудно придумать что-то говорящее)
изображение

Copy link
Contributor

Choose a reason for hiding this comment

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

да, coinData звучит дельно

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Перезалил

@Mama1emon Mama1emon force-pushed the feature/AND-5815_Add_AptosApi branch from 16676d8 to 192df95 Compare January 16, 2024 11:04
@Mama1emon Mama1emon merged commit b788994 into develop Jan 16, 2024
3 checks passed
@Mama1emon Mama1emon deleted the feature/AND-5815_Add_AptosApi branch January 16, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iMaks99 iMaks99 iMaks99 approved these changes

@Sateetas Sateetas Sateetas approved these changes

@roman-pv roman-pv Awaiting requested review from roman-pv

@iiiburnyiii iiiburnyiii Awaiting requested review from iiiburnyiii

@kozarezvlad kozarezvlad Awaiting requested review from kozarezvlad

@Yoggam1 Yoggam1 Awaiting requested review from Yoggam1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mama1emon @Sateetas @iMaks99