chore(Agents): Add agents#1994
Conversation
FabianDevel
added
documentation
PR Reviewer Guide 🔍(Review updated until commit 2aad25e)Here are some key observations to aid the review process:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ### Models | ||
| - File: `data/models/File.kt` | ||
| - Drive: `data/models/Drive.kt` | ||
| - User: `data/models/UiSettings.kt` |
There was a problem hiding this comment.
Suggestion: Correct the file path reference for the User model. The current reference to UiSettings.kt appears to be a copy-paste error from the line below it, as UiSettings typically stores UI preferences rather than User account data, which would mislead developers navigating the codebase. [general, importance: 6]
| - User: `data/models/UiSettings.kt` | |
| - User: `data/models/User.kt` |
FabianDevel
force-pushed
the
add-agents.md
branch
from
db8a4d2 to
2aad25e
Compare
|
Persistent review updated to latest commit 2aad25e |
|
|
||
| - File: `data/models/File.kt` | ||
| - Drive: `data/models/Drive.kt` | ||
| - User: `data/models/UiSettings.kt` |
There was a problem hiding this comment.
Suggestion: The User model is incorrectly mapped to UiSettings.kt, which likely contains UI preferences rather than user account data. This documentation error could mislead developers or AI agents attempting to locate the User data class. Update the reference to point to the actual User model file (typically data/models/User.kt or similar). [general, importance: 7]
| - User: `data/models/UiSettings.kt` | |
| - User: `data/models/User.kt` |
|
|
||
| ## Common Gotchas | ||
|
|
||
| - **Realm transactions**: Always use `realm.executeTransactionAwait()` with coroutines |
There was a problem hiding this comment.
Suggestion: The executeTransactionAwait() method belongs to the deprecated Realm Java Coroutines extension. Modern Realm Kotlin (io.realm.kotlin) uses realm.write { } for suspend functions. Update this guidance to prevent compilation errors when implementing Realm transactions with coroutines. [general, importance: 7]
| - **Realm transactions**: Always use `realm.executeTransactionAwait()` with coroutines | |
| - **Realm transactions**: Always use `realm.write { }` for suspend operations or `realm.writeBlocking { }` for synchronous operations |
| ### Sync & Services | ||
|
|
||
| - Sync adapter: `data/sync/SyncAdapter.kt` | ||
| - Upload service: `data/services/UploadWorker.kt` |
There was a problem hiding this comment.
Suggestion: The entry incorrectly labels a WorkManager Worker as a Service. UploadWorker extends Worker or CoroutineWorker, not Service. Correct the terminology to prevent confusion when developers search for background upload components using the wrong architectural pattern. [general, importance: 6]
| - Upload service: `data/services/UploadWorker.kt` | |
| - Upload worker: `data/services/UploadWorker.kt` |
|
This PR/issue depends on:
|
FabianDevel
force-pushed
the
add-agents.md
branch
from
2aad25e to
4f749c4
Compare
|
There was a problem hiding this comment.
Pull request overview
Adds agent-facing documentation (AGENTS.md) at the repository root and for the app/ module to help contributors quickly understand structure, conventions, and common commands.
Changes:
- Add root-level
AGENTS.mdwith project snapshot, setup commands, conventions, and module pointers. - Add
app/AGENTS.mddescribing app architecture, key files, quick search commands, and testing notes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| AGENTS.md | Root “agent guide” with project-wide setup/conventions and links to module-specific guides. |
| app/AGENTS.md | App-module-specific guide covering architecture, key paths, commands, and gotchas. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### Realm Models | ||
|
|
||
| - **DO**: Use `@RealmClass` annotation, open classes |
There was a problem hiding this comment.
This section says Realm models should use @RealmClass, but data/models/File.kt (the provided example) is an open class ... : RealmObject() without @RealmClass. Please reword to match actual usage (e.g., open class : RealmObject() and @RealmClass only where needed, such as embedded objects).
| - **DO**: Use `@RealmClass` annotation, open classes | |
| - **DO**: Define Realm models as `open class ... : RealmObject()` | |
| - Use `@RealmClass` only where needed (for example, embedded Realm objects) |
|
|
||
| ### Sync & Services | ||
|
|
||
| - Sync adapter: `data/sync/SyncAdapter.kt` |
There was a problem hiding this comment.
data/sync/SyncAdapter.kt is referenced here, but app/src/main/java/com/infomaniak/drive/data/sync/ currently contains workers (e.g., MediaObserverWorker.kt) and no SyncAdapter.kt. Please update this reference to an existing worker/entry point for sync.
| - Sync adapter: `data/sync/SyncAdapter.kt` | |
| - Sync worker: `data/sync/MediaObserverWorker.kt` |
|
|
||
| ### String resources | ||
|
|
||
| - After every change in strings.xml files, raise a warning for the user that he MUST mirror the changes to Loco |
There was a problem hiding this comment.
Wording: “raise a warning for the user that he MUST …” is unclear (who raises the warning?) and uses gendered language. Please rephrase to a concrete action (e.g., add a PR checklist item) and use neutral wording (“they/the user”, avoid all-caps MUST unless it’s an enforced rule).
| - After every change in strings.xml files, raise a warning for the user that he MUST mirror the changes to Loco | |
| - After every change in `strings.xml` files, remind the user to mirror the changes to Loco |
|
|
||
| Modular Android application for kDrive cloud storage by Infomaniak. Uses MVVM architecture with Hilt DI, Realm for offline data, Ktor for API calls. Supports 3 build flavors (standard, fdroid, preprod) via Gradle composite builds with Infomaniak Core library. | ||
|
|
||
| **Languages**: Kotlin (100%), XML layouts (migrating to Compose) |
There was a problem hiding this comment.
This says “Kotlin (100%)” but the project also contains XML resources/layouts (and is “migrating to Compose”). Please adjust the wording to avoid a factual inconsistency (e.g., “Mostly Kotlin” / “Primary language: Kotlin”).
| **Languages**: Kotlin (100%), XML layouts (migrating to Compose) | |
| **Languages**: Primary language: Kotlin, with XML layouts/resources (migrating to Compose) |
| ## Security & Secrets | ||
|
|
||
| - Never commit `.env`, `local.properties`, or API keys to repository | ||
| - Drive API tokens stored in Room database (encrypted) |
There was a problem hiding this comment.
“Drive API tokens stored in Room database (encrypted)” is too specific for what’s visible in this repo: the app uses Core’s UserDatabase (Room) for users/tokens, but the encryption aspect isn’t verifiable here. Please either link to the Core implementation that enforces encryption or soften the claim to avoid misinformation.
| - Drive API tokens stored in Room database (encrypted) | |
| - Drive API tokens stored in Core's Room-backed user database |
| ### Module Structure | ||
|
|
||
| - **Main App**: `app/` → [see app/AGENTS.md](app/AGENTS.md) | ||
| - **Core Libraries**: `Core/` → [see Core/AGENTS.md](Core/AGENTS.md) |
There was a problem hiding this comment.
This links to Core/AGENTS.md, but Core is a git submodule and this repository snapshot doesn’t contain that file. Unless this PR also bumps the Core submodule to a commit that adds Core/AGENTS.md, the link will be broken; please ensure the submodule reference is updated (or adjust/remove the link until it exists).
| - **Core Libraries**: `Core/` → [see Core/AGENTS.md](Core/AGENTS.md) | |
| - **Core Libraries**: `Core/` → submodule contents live under `Core/` |
|
|
||
| ## Common Gotchas | ||
|
|
||
| - **Realm transactions**: Always use `realm.executeTransactionAwait()` with coroutines |
There was a problem hiding this comment.
realm.executeTransactionAwait() is mentioned as the required pattern, but this symbol doesn’t exist anywhere in this repository (no matches in source). Please update this “Realm transactions” gotcha to the actual coroutine-friendly transaction API used in the app (or add the missing helper if it’s intended).
| - **Realm transactions**: Always use `realm.executeTransactionAwait()` with coroutines | |
| - **Realm transactions**: Use the coroutine-friendly Realm write/transaction pattern already established in the surrounding app code; do not reference `realm.executeTransactionAwait()` unless that helper is actually added to the project |
|
|
||
| - **DO**: Use repository pattern with Ktor | ||
| - Example: `data/api/ApiRepository.kt` | ||
| - Wrapper: `data/api/ApiService.kt` |
There was a problem hiding this comment.
ApiService.kt is referenced as an API wrapper, but there is no data/api/ApiService.kt in this module (only ApiRepository.kt, ApiRoutes.kt, etc.). Please update the reference to an existing entry point or add the missing wrapper file so the guide doesn’t point readers to a non-existent path.
| - Wrapper: `data/api/ApiService.kt` | |
| - Entry point: `data/api/ApiRepository.kt` |
|
|
||
| - File: `data/models/File.kt` | ||
| - Drive: `data/models/Drive.kt` | ||
| - User: `data/models/UiSettings.kt` |
There was a problem hiding this comment.
The “Models” list labels UiSettings.kt as “User”, but the app’s user model comes from Core (com.infomaniak.core.auth.models.user.User) and UiSettings is preference storage. Please point “User” to the correct type/file (or rename this entry) to avoid misleading new contributors.
| - User: `data/models/UiSettings.kt` | |
| - UI settings: `data/models/UiSettings.kt` | |
| - User (Core): `com.infomaniak.core.auth.models.user.User` |
Depends on Infomaniak/android-core#736