Update README.md with current state, features, and build instructions#1380
Update README.md with current state, features, and build instructions#1380igordanilcenko wants to merge 1 commit intodevelopmentfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly updates the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides a comprehensive update to the README.md file, significantly improving its clarity, accuracy, and usefulness for developers. The new sections on the current state, build instructions, secrets management, and architecture are excellent additions. I've found a few minor inaccuracies in the documentation that should be addressed. Overall, this is a great improvement.
| on your device. | ||
| ### Requirements | ||
|
|
||
| - **Android Studio** — latest stable version (the project uses AGP 8.11.0 and Kotlin 2.2.0; older Studio versions will not work) |
There was a problem hiding this comment.
The specified Android Gradle Plugin version 8.11.0 and Kotlin version 2.2.0 appear to be incorrect, as these are not valid released versions. This could cause confusion for developers during project setup. Please either correct the version numbers or remove them to avoid ambiguity.
| - **Android Studio** — latest stable version (the project uses AGP 8.11.0 and Kotlin 2.2.0; older Studio versions will not work) | |
| - **Android Studio** — latest stable version (older Studio versions will not work) |
| 1. Comment out `preBuild.dependsOn("downloadLibwallet")` in `app/build.gradle.kts`. | ||
| 2. Place your `.a` static library files in `libwallet/arm64-v8a/` and `libwallet/x86_64/`, and the header at `libwallet/wallet.h`. | ||
|
|
||
| For updating OpenSSL for Android: https://github.com/217heidai/openssl_for_android/releases |
There was a problem hiding this comment.
The documentation points to a third-party GitHub repository for OpenSSL binaries. Relying on unofficial, pre-built binaries for a critical security library like OpenSSL can introduce a supply chain risk. It is recommended to either use an official distribution or build OpenSSL from source as part of the project's build process to ensure the integrity and security of the dependency.
|
|
||
| **Exolix exchange flow** — The two directions have slightly different flows: | ||
| - **Buy XTM** (external → XTM): `Exchange → SendFunds` (creates transaction, shows deposit QR) → `ExchangeStatus` | ||
| - **Sell XTM** (XTM → external): `Exchange → Review` (sends XTM via FFI wallet) → `SendFunds` → `ExchangeStatus` |
There was a problem hiding this comment.
The documented flow for a 'Sell XTM' transaction appears to be incorrect. The current documentation states the flow is Exchange → Review → SendFunds → ExchangeStatus. However, the implementation in ExchangeReviewViewModel shows that after the 'Review' step, the application navigates directly to ExchangeStatus. The SendFunds step is not part of this flow. Please correct the documentation to reflect the actual implementation.
| - **Sell XTM** (XTM → external): `Exchange → Review` (sends XTM via FFI wallet) → `SendFunds` → `ExchangeStatus` | |
| - **Sell XTM** (XTM → external): `Exchange → Review` (sends XTM via FFI wallet) → `ExchangeStatus` |
There was a problem hiding this comment.
Pull request overview
Updates project documentation and configuration templates to reflect the current (v1.5.0) state of the Android wallet, including the Exolix exchange feature and modernized build/setup instructions.
Changes:
- Expanded
README.mdwith current-state feature list, updated build requirements/steps, secrets/configuration guidance, and architecture overview. - Updated
secret-example.propertiesto include the Exolix API key placeholder and related comments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| README.md | Adds/upgrades documentation for features, build variants, secrets/config, FFI library handling, and architecture. |
| secret-example.properties | Adds Exolix API key template entry and improves readability with spacing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Exolix API key for the crypto swap (exchange) feature. | ||
| # Obtain your key by registering at https://exolix.com/developers | ||
| # Leave empty to disable the exchange feature (UI will still appear but API calls will fail). |
There was a problem hiding this comment.
The Exolix API-key comment is internally inconsistent: it says leaving the key empty will "disable" the exchange feature, but then states the UI will still appear and API calls will fail. Please reword to accurately reflect the behavior (either the feature is hidden/disabled, or it remains visible but non-functional without a key).
| # Leave empty to disable the exchange feature (UI will still appear but API calls will fail). | |
| # Leave empty if you don't have a key; the exchange UI will still appear but swaps will not work (API calls will fail). |
|
|
||
| ## Architecture Overview | ||
|
|
||
| The project follows a single-module MVVM architecture with Dagger 2 for dependency injection. Screens are implemented as Fragments (newer ones in Jetpack Compose, older ones in XML view binding), all hosted inside `CommonActivity` subclasses. |
There was a problem hiding this comment.
This section calls the app a "single-module" architecture, but the project is configured as a multi-module Gradle build (e.g., :app and :yatlib are included in settings.gradle.kts). Please adjust wording to avoid misleading contributors (e.g., "primarily MVVM in the :app module" / "multi-module with app + yatlib").
| The project follows a single-module MVVM architecture with Dagger 2 for dependency injection. Screens are implemented as Fragments (newer ones in Jetpack Compose, older ones in XML view binding), all hosted inside `CommonActivity` subclasses. | |
| The project is a multi-module Gradle build (for example, `:app` and `:yatlib`), with the `:app` module primarily following an MVVM architecture and using Dagger 2 for dependency injection. Screens are implemented as Fragments (newer ones in Jetpack Compose, older ones in XML view binding), all hosted inside `CommonActivity` subclasses. |
| The download task (`downloadLibwallet`) runs as part of `preBuild` and places binaries into `libwallet/`. **Any files you manually put there will be overwritten.** | ||
|
|
||
| To use a custom (locally-built) native library: | ||
| 1. Comment out `preBuild.dependsOn("downloadLibwallet")` in `app/build.gradle.kts`. |
There was a problem hiding this comment.
The instructions for using a custom FFI library say to comment out preBuild.dependsOn("downloadLibwallet"), but app/build.gradle.kts actually wires the task via tasks.named("preBuild") { dependsOn("downloadLibwallet") }. Please update the README to match the current Gradle Kotlin DSL so readers can follow it verbatim.
| 1. Comment out `preBuild.dependsOn("downloadLibwallet")` in `app/build.gradle.kts`. | |
| 1. Comment out the line wiring the download task, `tasks.named("preBuild") { dependsOn("downloadLibwallet") }`, in `app/build.gradle.kts`. |
|
|
||
| **Compose design system** — All Compose screens must be wrapped in `TariDesignSystem(theme = ...) { ... }`. Access tokens via `TariDesignSystem.colors`, `TariDesignSystem.typography`, `TariDesignSystem.shapes`. | ||
|
|
||
| **Preference repositories** — Extend `CommonPrefRepository`. Keys are namespaced per active network via `TariNetwork.formatKey(key)`. Use `SharedPrefGsonDelegate` for serialised objects or `SharedPrefDelegate` for primitives. |
There was a problem hiding this comment.
The preferences guidance mentions a SharedPrefDelegate for primitives, but the codebase uses type-specific delegates (e.g., SharedPrefStringDelegate, SharedPrefBooleanDelegate, etc.) rather than a single SharedPrefDelegate. Please update this bullet to reference the actual delegate classes to avoid confusion.
| **Preference repositories** — Extend `CommonPrefRepository`. Keys are namespaced per active network via `TariNetwork.formatKey(key)`. Use `SharedPrefGsonDelegate` for serialised objects or `SharedPrefDelegate` for primitives. | |
| **Preference repositories** — Extend `CommonPrefRepository`. Keys are namespaced per active network via `TariNetwork.formatKey(key)`. Use `SharedPrefGsonDelegate` for serialised objects or type-specific delegates for primitives (e.g., `SharedPrefStringDelegate`, `SharedPrefBooleanDelegate`, etc.). |
| # This file must *NOT* be checked into Version Control Systems, | ||
| # as it contains sensitive app configuration data. |
There was a problem hiding this comment.
The header currently says this file must not be checked into version control, but secret-example.properties is intentionally committed as a template. Consider updating the wording to clarify that secret.properties (the generated file with real secrets) must not be committed, while the *-example.properties template is safe to keep in VCS.
| # This file must *NOT* be checked into Version Control Systems, | |
| # as it contains sensitive app configuration data. | |
| # This is an example/template configuration file that is safe to keep in Version Control Systems. | |
| # Do NOT commit the generated `secret.properties` file that contains your real sensitive app configuration data. |
No description provided.