Browse Source

Merge pull request #71 from vector-im/feature/bma/documentation

Update documentation
Johennes-patch-1
Benoit Marty 2 years ago committed by GitHub
parent
commit
972f99ad1a
  1. 17
      AUTHORS.md
  2. 158
      CONTRIBUTING.md
  3. 57
      README.md
  4. 235
      docs/_developer_onboarding.md
  5. 11
      docs/analytics.md
  6. 106
      docs/danger.md
  7. 143
      docs/design.md
  8. 49
      docs/installing_from_ci.md
  9. 131
      docs/integration_tests.md
  10. 52
      docs/nightly_build.md
  11. 284
      docs/notifications.md
  12. 290
      docs/pull_request.md
  13. 22
      docs/usefulLinks.md

17
AUTHORS.md

@ -0,0 +1,17 @@ @@ -0,0 +1,17 @@
A full developer contributors list can be found [here](https://github.com/vector-im/element-x-android/graphs/contributors).
# Core team:
The element.io Android developer team.
# Other contributors
First of all, we thank all contributors who use Element and report problems on this GitHub project or via the integrated rageshake function.
We do not forget all translators, for their work of translating Element into many languages. They are also the authors of Element.
Feel free to add your name below, when you contribute to the project!
Name | Matrix ID | GitHub
----------|-----------------------------|--------------------------------------
name | @name:matrix.org | [githubID](https://github.com/githubID)

158
CONTRIBUTING.md

@ -0,0 +1,158 @@ @@ -0,0 +1,158 @@
# Contributing to Element Android
<!--- TOC -->
* [Contributing code to Matrix](#contributing-code-to-matrix)
* [Android Studio settings](#android-studio-settings)
* [Compilation](#compilation)
* [I want to help translating Element](#i-want-to-help-translating-element)
* [I want to submit a PR to fix an issue](#i-want-to-submit-a-pr-to-fix-an-issue)
* [Kotlin](#kotlin)
* [Changelog](#changelog)
* [Code quality](#code-quality)
* [ktlint](#ktlint)
* [knit](#knit)
* [lint](#lint)
* [Unit tests](#unit-tests)
* [Tests](#tests)
* [Internationalisation](#internationalisation)
* [Accessibility](#accessibility)
* [Jetpack Compose](#jetpack-compose)
* [Authors](#authors)
* [Thanks](#thanks)
<!--- END -->
## Contributing code to Matrix
Please read https://github.com/matrix-org/synapse/blob/master/CONTRIBUTING.md
Element X Android support can be found in this room: [![Element Android Matrix room #element-android:matrix.org](https://img.shields.io/matrix/element-android:matrix.org.svg?label=%23element-android:matrix.org&logo=matrix&server_fqdn=matrix.org)](https://matrix.to/#/#element-android:matrix.org).
The rest of the document contains specific rules for Matrix Android projects
## Android Studio settings
Please set the "hard wrap" setting of Android Studio to 160 chars, this is the setting we use internally to format the source code (Menu `Settings/Editor/Code Style` then `Hard wrap at`).
Please ensure that you're using the project formatting rules (which are in the project at .idea/codeStyles/), and format the file before committing them.
## Compilation
This project should compile without any special action. Just clone it and open it with Android Studio, or compile from command line using `gradlew`.
## I want to help translating Element
For now strings are coming from Element Android project, so:
- If you want to fix an issue with an English string, please submit a PR on Element Android.
- If you want to fix an issue in other languages, or add a missing translation, or even add a new language, please use [Weblate](https://translate.element.io/projects/element-android/).
## I want to submit a PR to fix an issue
Please have a look in the [dedicated documentation](./docs/pull_request.md) about pull request.
Please check if a corresponding issue exists. If yes, please let us know in a comment that you're working on it.
If an issue does not exist yet, it may be relevant to open a new issue and let us know that you're implementing it.
### Kotlin
This project is full Kotlin. Please do not write Java classes.
### Changelog
Please create at least one file under ./changelog.d containing details about your change. Towncrier will be used when preparing the release.
Towncrier says to use the PR number for the filename, but the issue number is also fine.
Supported filename extensions are:
- ``.feature``: Signifying a new feature in Element Android or in the Matrix SDK.
- ``.bugfix``: Signifying a bug fix.
- ``.wip``: Signifying a work in progress change, typically a component of a larger feature which will be enabled once all tasks are complete.
- ``.doc``: Signifying a documentation improvement.
- ``.misc``: Any other changes.
See https://github.com/twisted/towncrier#news-fragments if you need more details.
### Code quality
Make sure the following commands execute without any error:
<pre>
./gradlew check
</pre>
Some separate commands can also be run, see below.
#### ktlint
<pre>
./gradlew ktlintCheck --continue
</pre>
Note that you can run
<pre>
./gradlew ktlintFormat
</pre>
For ktlint to fix some detected errors for you (you still have to check and commit the fix of course)
#### knit
[knit](https://github.com/Kotlin/kotlinx-knit) is a tool which checks markdown files on the project. Also it generates/updates the table of content (toc) of the markdown files.
So everytime the toc should be updated, just run
<pre>
./gradlew knit
</pre>
and commit the changes.
The CI will check that markdown files are up to date by running
<pre>
./gradlew knitCheck
</pre>
#### lint
<pre>
./gradlew lint
</pre>
### Unit tests
Make sure the following commands execute without any error:
<pre>
./gradlew test
</pre>
### Tests
Element X is currently supported on Android Lollipop (API 21+): please test your change on an Android device (or Android emulator) running with API 21. Many issues can happen (including crashes) on older devices.
Also, if possible, please test your change on a real device. Testing on Android emulator may not be sufficient.
You should consider adding Unit tests with your PR, and also integration tests (AndroidTest). Please refer to [this document](./docs/integration_tests.md) to install and run the integration test environment.
### Internationalisation
For now strings are coming from Element Android project, so please read [the documentation](https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#internationalisation) from there.
### Accessibility
Please consider accessibility as an important point. As a minimum requirement, in layout XML files please use attributes such as `android:contentDescription` and `android:importantForAccessibility`, and test with a screen reader if it's working well. You can add new string resources, dedicated to accessibility, in this case, please prefix theirs id with `a11y_`.
For instance, when updating the image `src` of an ImageView, please also consider updating its `contentDescription`. A good example is a play pause button.
### Jetpack Compose
When adding or editing `@Composable`, make sure that you create a `@Preview` function, with suffix `Preview`. This will also create a UI test automatically.
### Authors
Feel free to add an entry in file AUTHORS.md
## Thanks
Thanks for contributing to Matrix projects!

57
README.md

@ -6,30 +6,55 @@ @@ -6,30 +6,55 @@
[![Element Android Matrix room #element-android:matrix.org](https://img.shields.io/matrix/element-android:matrix.org.svg?label=%23element-android:matrix.org&logo=matrix&server_fqdn=matrix.org)](https://matrix.to/#/#element-android:matrix.org)
[![Weblate](https://translate.element.io/widgets/element-android/-/svg-badge.svg)](https://translate.element.io/engage/element-android/?utm_source=widget)
# element-x-android-poc
# element-x-android
Proof Of Concept to run a Matrix client on Android devices using the Matrix Rust Sdk and Jetpack compose.
ElementX Android is a [Matrix](https://matrix.org/) Android Client provided by [Element](https://element.io/).
The plan is [here](https://github.com/vector-im/element-x-android-poc/issues/1)!
The application is a total rewrite of [Element-Android](https://github.com/vector-im/element-android) using the [Matrix Rust SDK](https://github.com/matrix-org/matrix-rust-sdk) underneath and targeting devices running Android 5+. The UI layer is written using Jetpack compose.
<!--- TOC -->
### Modules
* [Rust SDK](#rust-sdk)
* [Roadmap](#roadmap)
* [Contributing](#contributing)
* [Build instructions](#build-instructions)
* [Support](#support)
* [Copyright & License](#copyright-&-license)
This Android project is a multi modules project.
<!--- END -->
- `app` module is the Android application module. Other modules are libraries;
- `features` modules contain some UI and can be seen as screen of the application;
- `libraries` modules contain classes that can be useful for other modules to work.
## Rust SDK
A few details about some modules:
ElementX leverages the [Matrix Rust SDK](https://github.com/matrix-org/matrix-rust-sdk) through an FFI layer that the final client can directly import and use.
- `libraries-core` module contains utility classes;
- `libraries-designsystem` module contains Composables which can be used across the app (theme, etc.);
- `libraries-elementresources` module contains resource from Element Android (mainly strings);
- `libraries-matrix` module contains wrappers around the Matrix Rust SDK.
We're doing this as a way to share code between platforms and while we've seen promising results it's still in the experimental stage and bound to change.
Here is the current module dependency graph:
## Roadmap
<!-- To update this graph, run `./tools/docs/generateModuleGraph.sh` (one day the CI will do it hopefully). -->
<img src=./docs/images/module_graph.png width=800 />
We are aiming to have a fast and fully functional personal messaging application by the end of year 2023.
## Contributing
Please see our [contribution guide](CONTRIBUTING.md).
Come chat with the community in the dedicated Matrix [room](https://matrix.to/#/#element-android:matrix.org).
## Build instructions
Just clone the project and open it in Android Studio.
## Support
When you are experiencing an issue on ElementX Android, please first search in [GitHub issues](https://github.com/vector-im/element-x-android/issues)
and then in [#element-android:matrix.org](https://matrix.to/#/#element-android:matrix.org).
If after your research you still have a question, ask at [#element-android:matrix.org](https://matrix.to/#/#element-android:matrix.org). Otherwise feel free to create a GitHub issue if you encounter a bug or a crash, by explaining clearly in detail what happened. You can also perform bug reporting (Rageshake) from the Element application by shaking your phone or going to the application settings. This is especially recommended when you encounter a crash.
## Copyright & License
Copyright (c) 2022 New Vector Ltd
Licensed under the Apache License, Version 2.0 (the "License"); you may not use this work except in compliance with the License. You may obtain a copy of the License in the [LICENSE](LICENSE) file, or at:
[http://www.apache.org/licenses/LICENSE-2.0](http://www.apache.org/licenses/LICENSE-2.0)
Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.

235
docs/_developer_onboarding.md

@ -0,0 +1,235 @@ @@ -0,0 +1,235 @@
# Developer on boarding
<!--- TOC -->
* [Introduction](#introduction)
* [Quick introduction to Matrix](#quick-introduction-to-matrix)
* [Matrix data](#matrix-data)
* [Room](#room)
* [Event](#event)
* [Sync](#sync)
* [The Android project](#the-android-project)
* [Application](#application)
* [Jetpack Compose](#jetpack-compose)
* [Global architecture](#global-architecture)
* [Template](#template)
* [Push](#push)
* [Dependencies management](#dependencies-management)
* [Test](#test)
* [Other points](#other-points)
* [Logging](#logging)
* [Rageshake](#rageshake)
* [Tips](#tips)
* [Happy coding!](#happy-coding)
<!--- END -->
## Introduction
This doc is a quick introduction about the project and its architecture.
It's aim is to help new developers to understand the overall project and where to start developing.
Other useful documentation:
- all the docs in this folder!
- the [contributing doc](../CONTRIBUTING.md), that you should also read carefully.
### Quick introduction to Matrix
Matrix website: [matrix.org](https://matrix.org), [discover page](https://matrix.org/discover).
*Note*: Matrix.org is also hosting a homeserver ([.well-known file](https://matrix.org/.well-known/matrix/client)).
The reference homeserver (this is how Matrix servers are called) implementation is [Synapse](https://github.com/matrix-org/synapse/). But other implementations exist. The Matrix specification is here to ensure that any Matrix client, such as Element Android and its SDK can talk to any Matrix server.
Have a quick look to the client-server API documentation: [Client-server documentation](https://spec.matrix.org/v1.3/client-server-api/). Other network API exist, the list is here: (https://spec.matrix.org/latest/)
Matrix is an open source protocol. Change are possible and are tracked using [this GitHub repository](https://github.com/matrix-org/matrix-doc/). Changes to the protocol are called MSC: Matrix Spec Change. These are PullRequest to this project.
Matrix object are Json data. Unstable prefixes must be used for Json keys when the MSC is not merged (i.e. accepted).
#### Matrix data
There are many object and data in the Matrix worlds. Let's focus on the most important and used, `Room` and `Event`
##### Room
`Room` is a place which contains ordered `Event`s. They are identified with their `room_id`. Nearly all the data are stored in rooms, and shared using homeserver to all the Room Member.
*Note*: Spaces are also Rooms with a different `type`.
##### Event
`Events` are items of a Room, where data is embedded.
There are 2 types of Room Event:
- Regular Events: contain useful content for the user (message, image, etc.), but are not necessarily displayed as this in the timeline (reaction, message edition, call signaling).
- State Events: contain the state of the Room (name, topic, etc.). They have a non null value for the key `state_key`.
Also all the Room Member details are in State Events: one State Event per member. In this case, the `state_key` is the matrixId (= userId).
Important Fields of an Event:
- `event_id`: unique across the Matrix universe;
- `room_id`: the room the Event belongs to;
- `type`: describe what the Event contain, especially in the `content` section, and how the SDK should handle this Event;
- `content`: dynamic Event data; depends on the `type`.
So we have a triple `event_id`, `type`, `state_key` which uniquely defines an Event.
#### Sync
This is managed by the Rust SDK.
### The Android project
The project should compile out of the box.
This Android project is a multi modules project.
- `app` module is the Android application module. Other modules are libraries;
- `features` modules contain some UI and can be seen as screen or flow of screens of the application;
- `libraries` modules contain classes that can be useful for other modules to work.
A few details about some modules:
- `libraries-core` module contains utility classes;
- `libraries-designsystem` module contains Composables which can be used across the app (theme, etc.);
- `libraries-elementresources` module contains resource from Element Android (mainly strings);
- `libraries-matrix` module contains wrappers around the Matrix Rust SDK.
Most of the time a feature module should not know anything about other feature module.
The navigation glue is currently done in the `app` module.
Here is the current module dependency graph:
<!-- To update this graph, run `./tools/docs/generateModuleGraph.sh` (one day the CI will do it hopefully). -->
<img src=./images/module_graph.png width=800 />
### Application
This Android project mainly handle the application layer of the whole software. The communication with the Matrix server, as well as the local storage, the cryptography (encryption and decryption of Event, key management, etc.) is managed by the Rust SDK.
The application is responsible to store the session credentials though.
#### Jetpack Compose
Compose is essentially two libraries : Compose Compiler and Compose UI. The compiler (and his runtime) is actually not specific to UI at all and offer powerful
state management APIs. See https://jakewharton.com/a-jetpack-compose-by-any-other-name/
Some useful links:
- https://developer.android.com/jetpack/compose/mental-model
- https://developer.android.com/jetpack/compose/libraries
- https://developer.android.com/jetpack/compose/modifiers-list
- https://android.googlesource.com/platform/frameworks/support/+/androidx-main/compose/docs/compose-api-guidelines.md#api-guidelines-for-jetpack-compose
About Preview
- https://alexzh.com/jetpack-compose-preview/
#### Global architecture
Main libraries and frameworks used in this application:
- Navigation state with [Appyx](https://bumble-tech.github.io/appyx/)
- DI: [Dagger](https://dagger.dev/) and [Anvil](https://github.com/square/anvil)
- Reactive State management with Compose runtime and [Molecule](https://github.com/cashapp/molecule)
Some patterns are inspired by [Circuit](https://slackhq.github.io/circuit/)
Here are the main points:
1. `Presenter` and `View` does not communicate with each other directly, but through `State` and `Event`
2. Views are compose first
3. Presenters are also compose first, and have a single `present(): State` method. It's using the power of compose-runtime/compiler.
4. The point of connection between a `View` and a `Presenter` is a `Node`.
5. A `Node` is also responsible for managing Dagger components if any.
6. A `ParentNode` has some child `Node` and only know about them.
7. This is a single activity full compose application. The `MainActivity` is responsible for holding and configuring the `RootNode`.
8. There is no more needs for Android Architecture Component ViewModel as configuration change should be handled by Composable if needed.
#### Template
(TODO: This is coming)
There is a template module to easily start a new feature. When creating a new module, you can just copy paste the template.
### Push
**Note** Firebase Push is not yet implemented on the project.
Please see the dedicated [documentation](notifications.md) for more details.
This is the classical scenario:
- App receives a Push. Note: Push is ignored if app is in foreground;
- App asks the SDK to load Event data (fastlane mode). We have a change to get the data faster and display the notification faster;
- App asks the SDK to perform a sync request.
### Dependencies management
We are using [Gradle version catalog](https://docs.gradle.org/current/userguide/platforms.html#sub:central-declaration-of-dependencies) on this project.
All the dependencies (including android artifact, gradle plugin, etc.) should be declared in [../gradle/libs.versions.toml](libs.versions.toml) file.
Some dependency, mainly because they are not shared can be declared in `build.gradle.kts` files.
[Dependabot](https://github.com/dependabot) is set up on the project. This tool will automatically create Pull Request to upgrade our dependencies one by one.
**Note** Dependabot does not support yet Gradle verrsion catalog. This is tracked by [this issue](https://github.com/dependabot/dependabot-core/issues/3121).
### Test
We have 3 tests frameworks in place:
- Maestro to test the global usage of the application. See the related [documentation](../.maestro/README.md).
- Combination of [Showkase](https://github.com/airbnb/Showkase) and [Paparazzi](https://github.com/cashapp/paparazzi), to test UI pixel perfect. To add test, just add `@Preview` for the composable you are adding. See the related [documentation](screenshot_testing.md).
- Tests on presenter with Molecule and [Turbine](https://github.com/cashapp/turbine) (TODO this is coming)
**Note** For now we want to avoid using mock (such as *mockk*), because this should be note necessary.
### Other points
#### Logging
**Important warning: ** NEVER log private user data, or use the flag `LOG_PRIVATE_DATA`. Be very careful when logging `data class`, all the content will be output!
[Timber](https://github.com/JakeWharton/timber) is used to log data to logcat. We do not use directly the `Log` class. If possible please use a tag, as per
````kotlin
Timber.tag(loggerTag.value).d("my log")
````
because automatic tag (= class name) will not be available on the release version.
Also generally it is recommended to provide the `Throwable` to the Timber log functions.
Last point, note that `Timber.v` function may have no effect on some devices. Prefer using `Timber.d` and up.
#### Rageshake
Rageshake is a feature to send bug report directly from the application. Just shake your phone and you will be prompted to send a bug report.
Bug reports can contain:
- a screenshot of the current application state
- the application logs from up to 15 application starts
- the logcat logs
The data will be sent to an internal server, which is not publicly accessible. A GitHub issue will also be created to a private GitHub repository.
Rageshake can be very useful to get logs from a release version of the application.
### Tips
- Element Android has a `developer mode` in the `Settings/Advanced settings`. Other useful options are available here; (TODO Not supported yet!)
- Show hidden Events can also help to debug feature. When developer mode is enabled, it is possible to view the source (= the Json content) of any Events; (TODO Not supported yet!)
- Type `/devtools` in a Room composer to access a developer menu. There are some other entry points. Developer mode has to be enabled; (TODO Not supported yet!)
- Hidden debug menu: when developer mode is enabled and on debug build, there are some extra screens that can be accessible using the green wheel. In those screens, it will be possible to toggle some feature flags; (TODO Not supported yet!)
- Using logcat, filtering with `Compositions` can help you to understand what screen are currently displayed on your device. Searching for string displayed on the screen can also help to find the running code in the codebase.
- When this is possible, prefer using `sealed interface` instead of `sealed class`;
- When writing temporary code, using the string "DO NOT COMMIT" in a comment can help to avoid committing things by mistake. If committed and pushed, the CI will detect this String and will warn the user about it. (TODO Not supported yet!)
## Happy coding!
The team is here to support you, feel free to ask anything to other developers.
Also please feel to update this documentation, if incomplete/wrong/obsolete/etc.
**Thanks!**

11
docs/analytics.md

@ -0,0 +1,11 @@ @@ -0,0 +1,11 @@
# Analytics in Element
<!--- TOC -->
* [TODO](#todo)
<!--- END -->
## TODO
There is no analytics in the project yet.

106
docs/danger.md

@ -0,0 +1,106 @@ @@ -0,0 +1,106 @@
## Danger
<!--- TOC -->
* [What does danger checks](#what-does-danger-checks)
* [PR check](#pr-check)
* [Quality check](#quality-check)
* [Setup](#setup)
* [Run danger locally](#run-danger-locally)
* [Danger user](#danger-user)
* [Useful links](#useful-links)
<!--- END -->
## What does danger checks
### PR check
See the [dangerfile](../tools/danger/dangerfile.js). If you add rules in the dangerfile, please update the list below!
Here are the checks that Danger does so far:
- PR description is not empty
- Big PR got a warning to recommend to split
- PR contains a file for towncrier and extension is checked
- PR does not modify frozen classes
- PR contains a Sign-Off, with exception for Element employee contributors
- PR with change on layout should include screenshot in the description (TODO Not supported yet!)
- PR which adds png file warn about the usage of vector drawables
- non draft PR should have a reviewer
- files containing translations are not modified by developers
### Quality check
After all the checks that generate checkstyle XML report, such as Ktlint, lint, or Detekt, Danger is run with this [dangerfile](../tools/danger/dangerfile-lint.js), in order to post comments to the PR with the detected error and warnings.
To run locally, you will have to install the plugin `danger-plugin-lint-report` using:
```shell
yarn add danger-plugin-lint-report --dev
```
## Setup
This operation should not be necessary, since Danger is already setup for the project.
To setup danger to the project, run:
```shell
bundle exec danger init
```
## Run danger locally
When modifying the [dangerfile](../tools/danger/dangerfile.js), you can check it by running Danger locally.
To run danger locally, install it and run:
```shell
bundle exec danger pr <PR_URL> --dangerfile=./tools/danger/dangerfile.js
```
For instance:
```shell
bundle exec danger pr https://github.com/vector-im/element-android/pull/6637 --dangerfile=./tools/danger/dangerfile.js
```
We may need to create a GitHub token to have less API rate limiting, and then set the env var:
```shell
export DANGER_GITHUB_API_TOKEN='YOUR_TOKEN'
```
Swift and Kotlin (just in case)
```shell
bundle exec danger-swift pr <PR_URL> --dangerfile=./tools/danger/dangerfile.js
bundle exec danger-kotlin pr <PR_URL> --dangerfile=./tools/danger/dangerfile.js
```
## Danger user
To let Danger check all the PRs, including PRs form forks, a GitHub account have been created:
- login: ElementBot
- password: Stored on Passbolt
- GitHub token: A token with limited access has been created and added to the repository https://github.com/vector-im/element-android as secret DANGER_GITHUB_API_TOKEN. This token is not saved anywhere else. In case of problem, just delete it and create a new one, then update the secret.
PRs from forks do not always have access to the secret `secrets.DANGER_GITHUB_API_TOKEN`, so `secrets.GITHUB_TOKEN` is also provided to the job environment. If `secrets.DANGER_GITHUB_API_TOKEN` is available, it will be used, so user `ElementBot` will comment the PR. Else `secrets.GITHUB_TOKEN` will be used, and bot `github-actions` will comment the PR.
## Useful links
- https://danger.systems/
- https://danger.systems/js/
- https://danger.systems/js/guides/getting_started.html
- https://danger.systems/js/reference.html
- https://github.com/danger/awesome-danger
Some danger files to get inspired from
- https://github.com/artsy/emission/blob/master/dangerfile.ts
- https://github.com/facebook/react-native/blob/master/bots/dangerfile.js
- https://github.com/apollographql/apollo-client/blob/master/config/dangerfile.ts
- https://github.com/styleguidist/react-styleguidist/blob/master/dangerfile.js
- https://github.com/storybooks/storybook/blob/master/dangerfile.js
- https://github.com/ReactiveX/rxjs/blob/master/dangerfile.js

143
docs/design.md

@ -0,0 +1,143 @@ @@ -0,0 +1,143 @@
# Element Android design
<!--- TOC -->
* [Introduction](#introduction)
* [How to import from Figma to the Element Android project](#how-to-import-from-figma-to-the-element-android-project)
* [Colors](#colors)
* [Text](#text)
* [Dimension, position and margin](#dimension-position-and-margin)
* [Icons](#icons)
* [Export drawable from Figma](#export-drawable-from-figma)
* [Import in Android Studio](#import-in-android-studio)
* [Images](#images)
* [Figma links](#figma-links)
* [Coumpound](#coumpound)
* [Login](#login)
* [Login v2](#login-v2)
* [Room list](#room-list)
* [Timeline](#timeline)
* [Voice message](#voice-message)
* [Room settings](#room-settings)
* [VoIP](#voip)
* [Presence](#presence)
* [Spaces](#spaces)
* [List to be continued...](#list-to-be-continued)
<!--- END -->
**TODO This documentation is a bit outdated and must be updated when we will set up the design components.**
## Introduction
Design at element.io is done using Figma - https://www.figma.com
## How to import from Figma to the Element Android project
Integration should be done using the Android development best practice, and should follow the existing convention in the code.
### Colors
Element Android already contains all the colors which can be used by the designer, in the module `ui-style`.
Some of them depend on the theme, so ensure to use theme attributes and not colors directly.
### Text
- click on a text on Figma
- on the right panel, information about the style and colors are displayed
- in Element Android, text style are already defined, generally you should not create new style
- apply the style and the color to the layout
### Dimension, position and margin
- click on an item on Figma
- dimensions of the item will be displayed.
- move the mouse to other items to get relative positioning, margin, etc.
### Icons
#### Export drawable from Figma
- click on the element to export
- ensure that the correct layer is selected. Sometimes the parent layer has to be selected on the left panel
- on the right panel, click on "export"
- select SVG
- you can check the preview of what will be exported
- click on "export" and save the file locally
- unzip the file if necessary
It's also possible for any icon to go to the main component by right-clicking on the icon.
#### Import in Android Studio
- right click on the drawable folder where the drawable will be created
- click on "New"/"Vector Asset"
- select the exported file
- update the filename if necessary
- click on "Next" and click on "Finish"
- open the created vector drawable
- optionally update the color(s) to "#FF0000" (red) to ensure that the drawable is correctly tinted at runtime.
### Images
Android 4.3 (18+) fully supports the WebP image format which can often provide smaller image sizes without drastically impacting image quality (depending on the output encoding quality).
When importing non vector images, WebP is the preferred format.
Images can be converted to the WebP within Android Studio by
- right clicking the image file within the project file explorer
- select `Convert to WebP`
https://developer.android.com/studio/write/convert-webp
## Figma links
Figma links can be included in the layout, for future reference, but it is also OK to add a paragraph below here, to centralize the information
Main entry point: https://www.figma.com/files/project/5612863/Element?fuid=779371459522484071
Note: all the Figma links are not publicly available.
### Coumpound
Coumpound contains the theme of the application, with all the components, in Light and Dark theme: palette (colors), typography, iconography, etc.
https://www.figma.com/file/X4XTH9iS2KGJ2wFKDqkyed/Compound
### Login
TBD
#### Login v2
https://www.figma.com/file/xdV4PuI3DlzA1EiBvbrggz/Login-Flow-v2
### Room list
TBD
### Timeline
https://www.figma.com/file/x1HYYLYMmbYnhfoz2c2nGD/%5BRiotX%5D-Misc?node-id=0%3A1
### Voice message
https://www.figma.com/file/uaWc62Ux2DkZC4OGtAGcNc/Voice-Messages?node-id=473%3A12
### Room settings
TBD
### VoIP
https://www.figma.com/file/V6m2z0oAtUV1l8MdyIrAep/VoIP?node-id=4254%3A25767
### Presence
https://www.figma.com/file/qmvEskET5JWva8jZJ4jX8o/Presence---User-Status?node-id=114%3A9174
(Option B is chosen)
### Spaces
https://www.figma.com/file/m7L63aGPW7iHnIYStfdxCe/Spaces?node-id=192%3A30161
### List to be continued...

49
docs/installing_from_ci.md

@ -0,0 +1,49 @@ @@ -0,0 +1,49 @@
## Installing from CI
<!--- TOC -->
* [Installing from GitHub](#installing-from-github)
* [Create a GitHub token](#create-a-github-token)
* [Provide artifact URL](#provide-artifact-url)
* [Next steps](#next-steps)
* [Future improvement](#future-improvement)
<!--- END -->
Installing APK build by the CI is possible
### Installing from GitHub
TODO Import the script from Element Android and make it work, then update this documentation.
To install an APK built by a GitHub action, run the script `./tools/install/installFromGitHub.sh`. You will need to pass a GitHub token to do so.
#### Create a GitHub token
You can create a GitHub token going to your Github account, at this page: [https://github.com/settings/tokens](https://github.com/settings/tokens).
You need to create a token (classic) with the scope `repo/public_repo`. So just check the corresponding checkbox.
Validity can be long since the scope of this token is limited. You will still be able to delete the token and generate a new one.
Click on Generate token and save the token locally.
### Provide artifact URL
The script will ask for an artifact URL. You can get this artifact URL by following these steps:
- open the pull request
- in the check at the bottom, click on `APK Build / Build debug APKs`
- click on `Summary`
- scroll to the bottom of the page
- copy the link `vector-Fdroid-debug` if you want the F-Droid variant or `vector-Gplay-debug` if you want the Gplay variant.
The copied link can be provided to the script.
### Next steps
The script will download the artifact, unzip it and install the correct version (regarding arch) on your device.
Files will be added to the folder `./tmp/DebugApks`. Feel free to cleanup this folder from time to time, the script will not delete files.
### Future improvement
The script could ask the user for a Pull Request number and Gplay/Fdroid choice like it was done with Buildkite script. Using GitHub API may be possible to do that.

131
docs/integration_tests.md

@ -0,0 +1,131 @@ @@ -0,0 +1,131 @@
# Integration tests
<!--- TOC -->
* [Pre requirements](#pre-requirements)
* [Install and run Synapse](#install-and-run-synapse)
* [Run the test](#run-the-test)
* [Stop Synapse](#stop-synapse)
* [Troubleshoot](#troubleshoot)
* [Android Emulator does cannot reach the homeserver](#android-emulator-does-cannot-reach-the-homeserver)
* [Tests partially run but some fail with "Unable to contact localhost:8080"](#tests-partially-run-but-some-fail-with-"unable-to-contact-localhost:8080")
* [virtualenv command fails](#virtualenv-command-fails)
<!--- END -->
Integration tests are useful to ensure that the code works well for any use cases.
They can also be used as sample on how to use the Matrix SDK.
In a ideal world, every API of the SDK should be covered by integration tests. For the moment, we have test mainly for the Crypto part, which is the tricky part. But it covers quite a lot of features: accounts creation, login to existing account, send encrypted messages, keys backup, verification, etc.
The Matrix SDK is able to open multiple sessions, for the same user, of for different users. This way we can test communication between several sessions on a single device.
## Pre requirements
Integration tests need a homeserver running on localhost.
The documentation describes what we do to have one, using [Synapse](https://github.com/matrix-org/synapse/), which is the Matrix reference homeserver.
## Install and run Synapse
Steps:
- Install virtualenv
```bash
python3 -m pip install virtualenv
```
- Clone Synapse repository
```bash
git clone -b develop https://github.com/matrix-org/synapse.git
```
or
```bash
git clone -b develop git@github.com:matrix-org/synapse.git
```
You should have the develop branch cloned by default.
- Run synapse, from the Synapse folder you just cloned
```bash
virtualenv -p python3 env
source env/bin/activate
pip install -e .
demo/start.sh --no-rate-limit
```
Alternatively, to install the latest Synapse release package (and not a cloned branch) you can run the following instead of `git clone` and `pip install -e .`:
```bash
pip install matrix-synapse
```
On your first run, you will want to stop the demo and edit the config to correct the `public_baseurl` to http://10.0.2.2:8080 and restart the server.
You should now have 3 running federated Synapse instances 🎉, at http://127.0.0.1:8080/, http://127.0.0.1:8081/ and http://127.0.0.1:8082/, which should display a "It Works! Synapse is running" message.
## Run the test
It's recommended to run tests using an Android Emulator and not a real device. First reason for that is that the tests will use http://10.0.2.2:8080 to connect to Synapse, which run locally on your machine.
You can run all the tests in the `androidTest` folders.
It can be done using this command:
```bash
./gradlew vector:connectedAndroidTest matrix-sdk-android:connectedAndroidTest
```
## Stop Synapse
To stop Synapse, you can run the following commands:
```bash
./demo/stop.sh
```
And you can deactivate the virtualenv:
```bash
deactivate
```
## Troubleshoot
You'll need python3 to be able to run synapse
### Android Emulator does cannot reach the homeserver
Try on the Emulator browser to open "http://10.0.2.2:8080". You should see the "Synapse is running" message.
### Tests partially run but some fail with "Unable to contact localhost:8080"
This is because the `public_baseurl` of synapse is not consistent with the endpoint that the tests are connecting to.
Ensure you have the following configuration in `demo/etc/8080.config`.
```
public_baseurl: http://10.0.2.2:8080/
```
After changing this you will need to restart synapse using `demo/stop.sh` and `demo/start.sh` to load the new configuration.
### virtualenv command fails
You can try using
```bash
python3 -m venv env
```
or
```bash
python3 -m virtualenv env
```
instead of
```bash
virtualenv -p python3 env
```

52
docs/nightly_build.md

@ -0,0 +1,52 @@ @@ -0,0 +1,52 @@
# Nightly builds
<!--- TOC -->
* [Configuration](#configuration)
* [How to register to get nightly build](#how-to-register-to-get-nightly-build)
* [Build nightly manually](#build-nightly-manually)
<!--- END -->
## Configuration
The nightly build will contain what's on develop, in release mode, for the main variant. It is signed using a dedicated signature, and has a dedicated appId (`io.element.android.x.nightly`), so it can be installed along with the production version of ElementX Android. The only other difference compared to ElementX Android is a different app name. We do not want to change the app name since it will also affect some strings in the app, and we do want to do that. (TODO today, the app name is changed.)
Nightly builds are built and released to Firebase every days, and automatically.
This is recommended to exclusively use this app, with your main account, instead of ElementX Android, and fallback to ElementX Android just in case of regression, to discover as soon as possible any regression, and report it to the team. To avoid double notification, you may want to disable the notification from the Element Android production version. Just open Element Android, navigate to `Settings/Notifications` and uncheck `Enable notifications for this session` (TODO Not supported yet).
*Note:* Due to a limitation of Firebase, the nightly build is the universal build, which means that the size of the APK is a bit bigger, but this should not have any other side effect.
## How to register to get nightly build
Click on this link and follow the instruction: [https://appdistribution.firebase.dev/i/7de2dbc61e7fb2a6](https://appdistribution.firebase.dev/i/7de2dbc61e7fb2a6)
## Build nightly manually
Nightly build can be built manually from your computer. You will need to retrieved some secrets from Passbolt and add them to your file `~/.gradle/gradle.properties`:
```
signing.element.nightly.storePassword=VALUE_FROM_PASSBOLT
signing.element.nightly.keyId=VALUE_FROM_PASSBOLT
signing.element.nightly.keyPassword=VALUE_FROM_PASSBOLT
```
You will also need to add the environment variable `FIREBASE_TOKEN`:
```sh
export FIREBASE_TOKEN=VALUE_FROM_PASSBOLT
```
Then you can run the following commands (which are also used in the file for [the GitHub action](../.github/workflows/nightly.yml)):
```sh
git checkout develop
mv towncrier.toml towncrier.toml.bak
sed 's/CHANGES\.md/CHANGES_NIGHTLY\.md/' towncrier.toml.bak > towncrier.toml
rm towncrier.toml.bak
yes n | towncrier build --version nightly
./gradlew assembleGplayNightly appDistributionUploadGplayNightly $CI_GRADLE_ARG_PROPERTIES
```
Then you can reset the change on the codebase.

284
docs/notifications.md

@ -0,0 +1,284 @@ @@ -0,0 +1,284 @@
This document aims to describe how Element android displays notifications to the end user. It also clarifies notifications and background settings in the app.
# Table of Contents
<!--- TOC -->
* [Prerequisites Knowledge](#prerequisites-knowledge)
* [How does a matrix client get a message from a homeserver?](#how-does-a-matrix-client-get-a-message-from-a-homeserver?)
* [How does a mobile app receives push notification](#how-does-a-mobile-app-receives-push-notification)
* [Push VS Notification](#push-vs-notification)
* [Push in the matrix federated world](#push-in-the-matrix-federated-world)
* [How does the homeserver know when to notify a client?](#how-does-the-homeserver-know-when-to-notify-a-client?)
* [Push vs privacy, and mitigation](#push-vs-privacy-and-mitigation)
* [Background processing limitations](#background-processing-limitations)
* [Element Notification implementations](#element-notification-implementations)
* [Requirements](#requirements)
* [Foreground sync mode (Gplay and F-Droid)](#foreground-sync-mode-gplay-and-f-droid)
* [Push (FCM) received in background](#push-fcm-received-in-background)
* [FCM Fallback mode](#fcm-fallback-mode)
* [F-Droid background Mode](#f-droid-background-mode)
* [Application Settings](#application-settings)
<!--- END -->
First let's start with some prerequisite knowledge
## Prerequisites Knowledge
### How does a matrix client get a message from a homeserver?
In order to get messages from a homeserver, a matrix client need to perform a ``sync`` operation.
`To read events, the intended flow of operation is for clients to first call the /sync API without a since parameter. This returns the most recent message events for each room, as well as the state of the room at the start of the returned timeline. `
The client need to call the `sync` API periodically in order to get incremental updates of the server state (new messages).
This mechanism is known as **HTTP long Polling**.
Using the **HTTP Long Polling** mechanism a client polls a server requesting new information.
The server *holds the request open until new data is available*.
Once available, the server responds and sends the new information.
When the client receives the new information, it immediately sends another request, and the operation is repeated.
This effectively emulates a server push feature.
The HTTP long Polling can be fine tuned in the **SDK** using two parameters:
* timeout (Sync request timeout)
* delay (Delay between each sync)
**timeout** is a server parameter, defined by:
```
The maximum time to wait, in milliseconds, before returning this request.`
If no events (or other data) become available before this time elapses, the server will return a response with empty fields.
By default, this is 0, so the server will return immediately even if the response is empty.
```
**delay** is a client preference. When the server responds to a sync request, the client waits for `delay`before calling a new sync.
When the Element Android app is open (i.e in foreground state), the default timeout is 30 seconds, and delay is 0.
### How does a mobile app receives push notification
Push notification is used as a way to wake up a mobile application when some important information is available and should be processed.
Typically in order to get push notification, an application relies on a **Push Notification Service** or **Push Provider**.
For example iOS uses APNS (Apple Push Notification Service).
Most of android devices relies on Google's Firebase Cloud Messaging (FCM).
> FCM has replaced Google Cloud Messaging (GCM - deprecated April 10 2018)
FCM will only work on android devices that have Google plays services installed
(In simple terms, Google Play Services is a background service that runs on Android, which in turn helps in integrating Google’s advanced functionalities to other applications)
De-Googlified devices need to rely on something else in order to stay up to date with a server.
There some cases when devices with google services cannot use FCM (network infrastructure limitations -firewalls-,
privacy and or independence requirement, source code licence)
### Push VS Notification
This need some disambiguation, because it is the source of common confusion:
*The fact that you see a notification on your screen does not mean that you have successfully configured your PUSH platform.*
Technically there is a difference between a push and a notification. A notification is what you see on screen and/or in the notification Menu/Drawer (in the top bar of the phone).
Notifications are not always triggered by a push (One can display a notification locally triggered by an alarm)
### Push in the matrix federated world
In order to send a push to a mobile, App developers need to have a server that will use the FCM APIs, and these APIs requires authentication!
This server is called a **Push Gateway** in the matrix world
That means that Element Android, a matrix client created by New Vector, is using a **Push Gateway** with the needed credentials (FCM API secret Key) in order to send push to the New Vector client.
If you create your own matrix client, you will also need to deploy an instance of a **Push Gateway** with the credentials needed to use FCM for your app.
On registration, a matrix client must tell its homeserver what Push Gateway to use.
See [Sygnal](https://github.com/matrix-org/sygnal/) for a reference implementation.
```
+--------------------+ +-------------------+
Matrix HTTP | | | |
Notification Protocol | App Developer | | Device Vendor |
| | | |
+-------------------+ | +----------------+ | | +---------------+ |
| | | | | | | | | |
| Matrix homeserver +-----> Push Gateway +------> Push Provider | |
| | | | | | | | | |
+-^-----------------+ | +----------------+ | | +----+----------+ |
| | | | | |
Matrix | | | | | |
Client/Server API + | | | | |
| | +--------------------+ +-------------------+
| +--+-+ |
| | <-------------------------------------------+
+---+ |
| | Provider Push Protocol
+----+
Mobile Device or Client
```
Recommended reading:
* https://thomask.sdf.org/blog/2016/12/11/riots-magical-push-notifications-in-ios.html
* https://matrix.org/docs/spec/client_server/r0.4.0.html#id128
### How does the homeserver know when to notify a client?
This is defined by [**push rules**](https://matrix.org/docs/spec/client_server/r0.4.0.html#push-rules-).
`A push rule is a single rule that states under what conditions an event should be passed onto a push gateway and how the notification should be presented (sound / importance).`
A homeserver can be configured with default rules (for Direct messages, group messages, mentions, etc.. ).
There are different kind of push rules, it can be per room (each new message on this room should be notified), it can also define a pattern that a message should match (when you are mentioned, or key word based).
Notifications have 2 'levels' (`highlighted = true/false sound = default/custom`). In Element these notifications level are reflected as Noisy/Silent.
**What about encrypted messages?**
Of course, content patterns matching cannot be used for encrypted messages server side (as the content is encrypted).
That is why clients are able to **process the push rules client side** to decide what kind of notification should be presented for a given event.
### Push vs privacy, and mitigation
As seen previously, App developers don't directly send a push to the end user's device, they use a Push Provider as intermediary. So technically this intermediary is able to read the content of what is sent.
App developers usually mitigate this by sending a `silent notification`, that is a notification with no identifiable data, or with an encrypted payload. When the push is received the app can then synchronise to it's server in order to generate a local notification.
### Background processing limitations
A mobile applications process live in a managed word, meaning that its process can be limited (e.g no network access), stopped or killed at almost anytime by the Operating System.
In order to improve the battery life of their devices some constructors started to implement mechanism to drastically limit background execution of applications (e.g MIUI/Xiaomi restrictions, Sony stamina mode).
Then starting android M, android has also put more focus on improving device performances, introducing several IDLE modes, App-Standby, Light Doze, Doze.
In a nutshell, apps can't do much in background now.
If the devices is not plugged and stays IDLE for a certain amount of time, radio (mobile connectivity) and CPU can/will be turned off.
For an application like Element, where users can receive important information at anytime, the best option is to rely on a push system (Google's Firebase Message a.k.a FCM). FCM high priority push can wake up the device and whitelist an application to perform background task (for a limited but unspecified amount of time).
Notice that this is still evolving, and in future versions application that has been 'background restricted' by users won't be able to wake up even when a high priority push is received. Also high priority notifications could be rate limited (not defined anywhere)
It's getting a lot more complicated when you cannot rely on FCM (because: closed sources, network/firewall restrictions, privacy concerns).
The documentation on this subject is vague, and as per our experiments not always exact, also device's behaviour is fragmented.
It is getting more and more complex to have reliable notifications when FCM is not used.
## Element Notification implementations
### Requirements
Element Android must work with and without FCM.
* The Element android app published on F-Droid do not rely on FCM (all related dependencies are not present)
* The Element android app published on google play rely on FCM, with a fallback mode when FCM registration has failed (e.g outdated or missing Google Play Services)
### Foreground sync mode (Gplay and F-Droid)
When in foreground, Element performs sync continuously with a timeout value set to 10 seconds (see HttpPooling).
As this mode does not need to live beyond the scope of the application, and as per Google recommendation, Element uses the internal app resources (Thread and Timers) to perform the syncs.
This mode is turned on when the app enters foreground, and off when enters background.
In background, and depending on whether push is available or not, Element will use different methods to perform the syncs (Workers / Alarms / Service)
### Push (FCM) received in background
In order to enable Push, Element must first get a push token from the firebase SDK, then register a pusher with this token on the homeserver.
When a message should be notified to a user, the user's homeserver notifies the registered `push gateway` for Element, that is [sygnal](https://github.com/matrix-org/sygnal) _- The reference implementation for push gateways -_ hosted by matrix.org.
This sygnal instance is configured with the required FCM API authentication token, and will then use the FCM API in order to notify the user's device running Element.
```
Homeserver ----> Sygnal (configured for Element) ----> FCM ----> Element
```
The push gateway is configured to only send `(eventId,roomId)` in the push payload (for better [privacy](#push-vs-privacy-and-mitigation)).
Element needs then to synchronise with the user's homeserver, in order to resolve the event and create a notification.
As per [Google recommendation](https://android-developers.googleblog.com/2018/09/notifying-your-users-with-fcm.html), Element will then use the WorkManager API in order to trigger a background sync.
**Google recommendations:**
> We recommend using FCM messages in combination with the WorkManager 1 or JobScheduler API
> Avoid background services. One common pitfall is using a background service to fetch data in the FCM message handler, since background service will be stopped by the system per recent changes to Google Play Policy
```
Homeserver ----> Sygnal ----> FCM ----> Element
(Sync) ----> Homeserver
<----
Display notification
```
**Possible outcomes**
Upon reception of the FCM push, Element will perform a sync call to the homeserver, during this process it is possible that:
* Happy path, the sync is performed, the message resolved and displayed in the notification drawer
* The notified message is not in the sync. Can happen if a lot of things did happen since the push (`gappy sync`)
* The sync generates additional notifications (e.g an encrypted message where the user is mentioned detected locally)
* The sync takes too long and the process is killed before completion, or network is not reliable and the sync fails.
Element implements several strategies in these cases (TODO document)
### FCM Fallback mode
It is possible that Element is not able to get a FCM push token.
Common errors (among several others) that can cause that:
* Google Play Services is outdated
* Google Play Service fails in someways with FCM servers (infamous `SERVICE_NOT_AVAILABLE`)
If Element is able to detect one of this cases, it will notifies it to the users and when possible help him fix it via a dedicated troubleshoot screen.
Meanwhile, in order to offer a minimal service, and as per Google's recommendation for background activities, Element will launch periodic background sync in order to stays in sync with servers.
The fallback mode is impacted by all the battery life saving mechanism implemented by android. Meaning that if the app is not used for a certain amount of time (`App-Standby`), or the device stays still and unplugged (`Light Doze`) , the sync will become less frequent.
And if the device stays unplugged and still for too long (`Doze Mode`), no background sync will be perform at all (the system's `Ignore Battery Optimization option` has no effect on that).
Also the time interval between sync is elastic, controlled by the system to group other apps background sync request and start radio/cpu only once for all.
Usually in this mode, what happen is when you take back your phone in your hand, you suddenly receive notifications.
The fallback mode is supposed to be a temporary state waiting for the user to fix issues for FCM, or for App Developers that has done a fork to correctly configure their FCM settings.
### F-Droid background Mode
The F-Droid Element flavor has no dependencies to FCM, therefore cannot relies on Push.
Also Google's recommended background processing method cannot be applied. This is because all of these methods are affected by IDLE modes, and will result on the user not being notified at all when the app is in a Doze mode (only in maintenance windows that could happens only after hours).
Only solution left is to use `AlarmManager`, that offers new API to allow launching some process even if the App is in IDLE modes.
Notice that these alarms, due to their potential impact on battery life, can still be restricted by the system. Documentation says that they will not be triggered more than every minutes under normal system operation, and when in low power mode about every 15 mn.
These restrictions can be relaxed by requiring the app to be white listed from battery optimization.
F-Droid version will schedule alarms that will then trigger a Broadcast Receiver, that in turn will launch a Service (in the classic android way), and the reschedule an alarm for next time.
Depending on the system status (or device make), it is still possible that the app is not given enough time to launch the service, or that the radio is still turned off thus preventing the sync to success (that's why Alarms are not recommended for network related tasks).
That is why on Element F-Droid, the broadcast receiver will acquire a temporary WAKE_LOCK for several seconds (thus securing cpu/network), and launch the service in foreground. The service performs the sync.
Note that foreground services require to put a notification informing the user that the app is doing something even if not launched).
## Application Settings
**Notifications > Enable notifications for this account**
Configure Sygnal to send or not notifications to all user devices.
**Notifications > Enable notifications for this device**
Disable notifications locally. The push server will continue to send notifications to the device but this one will ignore them.

290
docs/pull_request.md

@ -0,0 +1,290 @@ @@ -0,0 +1,290 @@
# Pull requests
<!--- TOC -->
* [Introduction](#introduction)
* [Who should read this document?](#who-should-read-this-document?)
* [Submitting PR](#submitting-pr)
* [Who can submit pull requests?](#who-can-submit-pull-requests?)
* [Humans](#humans)
* [Draft PR?](#draft-pr?)
* [Base branch](#base-branch)
* [PR Review Assignment](#pr-review-assignment)
* [PR review time](#pr-review-time)
* [Re-request PR review](#re-request-pr-review)
* [When create split PR?](#when-create-split-pr?)
* [Avoid fixing other unrelated issue in a big PR](#avoid-fixing-other-unrelated-issue-in-a-big-pr)
* [Bots](#bots)
* [Dependabot](#dependabot)
* [Gradle wrapper](#gradle-wrapper)
* [Sync analytics plan](#sync-analytics-plan)
* [Reviewing PR](#reviewing-pr)
* [Who can review pull requests?](#who-can-review-pull-requests?)
* [What to have in mind when reviewing a PR](#what-to-have-in-mind-when-reviewing-a-pr)
* [Rules](#rules)
* [Check the form](#check-the-form)
* [PR title](#pr-title)
* [PR description](#pr-description)
* [File change](#file-change)
* [Check the commit](#check-the-commit)
* [Check the substance](#check-the-substance)
* [Make a dedicated meeting to review the PR](#make-a-dedicated-meeting-to-review-the-pr)
* [What happen to the issue(s)?](#what-happen-to-the-issues?)
* [Merge conflict](#merge-conflict)
* [When and who can merge PR](#when-and-who-can-merge-pr)
* [Merge type](#merge-type)
* [Resolve conversation](#resolve-conversation)
* [Responsibility](#responsibility)
<!--- END -->
## Introduction
This document gives some clue about how to efficiently manage Pull Requests (PR). This document is a first draft and may be improved later.
## Who should read this document?
Every pull request reviewers, but also probably every ones who submit PRs.
## Submitting PR
### Who can submit pull requests?
Basically every one who wants to contribute to the project! But there are some rules to follow.
#### Humans
People with write access to the project can directly clone the project, push their branches and create PR.
External contributors must first fork the project and create PR to the mainline from there.
##### Draft PR?
Draft PR can be created when the submitter does not expect the PR to be reviewed and merged yet. It can be useful to publicly show the work, or to do a self-review first.
Draft PR can also be created when it depends on other un-merged PR.
In any case, it is better to explicitly declare in the description why the PR is a draft PR.
Also, draft PR should not stay indefinitely in this state. It may be removed if it is the case and the submitter does not update it after a few days.
##### Base branch
The `develop` branch is generally the base branch for every PRs.
Exceptions can occur:
- if a feature implementation is split into multiple PRs. We can have a chain of PRs in this case. PR can be merged one by one on develop, and GitHub change the target branch to `develop` for the next PR automatically.
- we want to merge a PR from the community, but there is still work to do, and the PR is not updated by the submitter. First, we can kindly ask the submitter if they will update their PR, by commenting it. If there is no answer after a few days (including a week-end), we can create a new branch, push it, and change the target branch of the PR to this new branch. The PR can then be merged, and we can add more commits to fix the issues. After that a new PR can be created with `develop` as a target branch.
**Important notice 1:** Releases are created from the `develop` branch. So `develop` branch should always contain a "releasable" source code. So when a feature is being implemented with several PRs, it has to be disabled by default (using a feature flag for instance), until the feature is fully implemented. A last PR to enable the feature can then be created.
**Important notice 2:** Database migration: some developers and some people from the community are using the nightly build from `develop`. Multiple database migrations should be properly handled for them. This is OK to have multiple migrations between 2 releases, this is not OK to add steps to the pending database migration on `develop`. So for instance `develop` users will migrate from version 11 to version 12, then 13, then 14, and `main` users will do all those steps after they get the app upgrade.
##### PR Review Assignment
We use automatic assignment for PR reviews. **A PR is automatically routed by GitHub to one team member** using the round robin algorithm. Additional reviewers can be used for complex changes or when the first reviewer is not confident enough on the changes.
The process is the following:
- The PR creator selects the [element-x-android-reviewers](https://github.com/orgs/vector-im/teams/element-x-android-reviewers) team as a reviewer.
- GitHub automatically assign the reviewer. If the reviewer is not available (holiday, etc.), remove them and set again the team, GitHub will select another reviewer.
- Alternatively, the PR creator can directly assign specific people if they have another Android developer in their team or they think a specific reviewer should take a look at their PR.
- Reviewers get a notification to make the review: they review the code following the good practice (see the rest of this document).
- After making their own review, if they feel not confident enough, they can ask another person for a full review, or they can tag someone within a PR comment to check specific lines.
For PRs coming from the community, the issue wrangler can assign either the team [element-x-android-reviewers](https://github.com/orgs/vector-im/teams/element-x-android-reviewers) or any member directly.
##### PR review time
As a PR submitter, you deserve a quick review. As a reviewer, you should do your best to unblock others.
Some tips to achieve it:
- Set up your GH notifications correctly
- Check your pulls page: [https://github.com/pulls](https://github.com/pulls)
- Check your pending assigned PRs before starting or resuming your day to day tasks
- If you are busy with high priority tasks, inform the author. They will find another developer
It is hard to define a deadline for a review. It depends on the PR size and the complexity. Let's start with a goal of 24h (working day!) for a PR smaller than 500 lines. If bigger, the submitter and the reviewer should discuss.
After this time, the submitter can ping the reviewer to get a status of the review.
##### Re-request PR review
Once all the remarks have been handled, it's possible to re-request a review from the (same) reviewer to let them know that the PR has been updated the PR is ready to be reviewed again. Use the double arrow next to the reviewer name to do that.
##### When create split PR?
To implement big new feature, it may be efficient to split the work into several smaller and scoped PRs. They will be easier to review, and they can be merged on `develop` faster.
Big PR can take time, and there is a risk of future merge conflict.
Feature flag can be used to avoid half implemented feature to be available in the application.
That said, splitting into several PRs should not have the side effect to have more review to do, for instance if some code is added, then finally removed.
##### Avoid fixing other unrelated issue in a big PR
Each PR should focus on a single task. If other issues may be fixed when working in the area of it, it's preferable to open a dedicated PR.
It will have the advantage to be reviewed and merged faster, and not interfere with the main PR.
It's also applicable for code rework (such as renaming for instance), or code formatting. Sometimes, it is more efficient to extract that work to a dedicated PR, and rebase your branch once this "rework" PR has been merged.
#### Bots
Some bots can create PR, but they still have to be reviewed by the team
##### Dependabot
Dependabot is a tool which maintain all our external dependencies up to date. A dedicated PR is created for each new available release for one of our external dependency.Dependabot
To review such PR, you have to
- **IMPORTANT** check the diff files (as always).
- Check the release note. Some existing bugs in Element project may be fixed by the upgrade
- Make sure that the CI is happy
- If the code does not compile (API break for instance), you have to checkout the branch and push new commits
- Do some smoke test, depending of the library which has been upgraded
For some reason dependabot sometimes does not upgrade some dependencies. In this case, and when detected, the upgrade has to be done manually.
##### Gradle wrapper
`Update Gradle Wrapper` is a tool which can create PR to upgrade our gradle.properties file.
Review such PR is the same recipe than for PR from Dependabot
##### Sync analytics plan
This tools imports any update in the analytics plan. See instruction in the PR itself to handle it.
More info can be found in the file [analytics.md](./analytics.md)
## Reviewing PR
### Who can review pull requests?
As an open source project, every one can review each others PR. Of course an approval from internal developer is mandatory for a PR to be merged.
But comment in PR from the community are always appreciated!
### What to have in mind when reviewing a PR
1. User experience: is the UX and UI correct? You will probably be the second person to test the new thing, the first one is the developer.
2. Developer experience: does the code look nice and decoupled? No big functions, new classes added to the right module, etc.
3. Code maintenance. A bit similar to point 2. Tricky code must be documented for instance
4. Fork consideration. Will configuration of forks be easy? Some documentation may help in some cases.
5. We are building long term products. "Quick and dirty" code must be avoided.
6. The PR includes new tests for the added code, updated test for the existing code
7. All PRs from external contributors **MUST** include a sign-off. It's in the checklist, and sometimes it's checked by the submitter, but there is actually no sign-off. In this case, ask nicely for a sign-off and request changes (do not approve the PR, even if everything else is fine).
### Rules
#### Check the form
##### PR title
PR title should describe in one line what's brought by the PR. Reviewer can edit the title if it's not clear enough, or to add suffix like `[BLOCKED]` or similar. Fixing typo is also a good practice, since GitHub search is quite not efficient, so the words must be spelled without any issue. Adding suffix will help when viewing the PR list.
It's free form, but prefix tags could also be used to help understand what's in the PR.
Examples of prefixes:
- `[Refacto]`
- `[Feature]`
- `[Bugfix]`
- etc.
Also, it's still possible to add labels to the PRs, such as `A-` or `T-` labels, even if this is not a strong requirement. We prefer to spend time to add labels on issues.
##### PR description
PR description should follow the PR template, and at least provide some context about the code change.
##### File change
1. Code should follow the guidelines
2. Code should be formatted correctly
3. XML attribute must be sorted
4. New code is added at the correct location
5. New classes are added to the correct location
6. Naming is correct. Naming is really important, it's considered part of the documentation
7. Architecture is followed. For instance, the logic is in the ViewModel and not in the Fragment
8. There is at least one file for the changelog. Exception if the PR fixes something which has not been released yet. Changelog content should target their audience: `.sdk` extension are mainly targeted for developers, other extensions are targeted for users and forks maintainers. It should generally describe visual change rather than give technical details. More details can be found [here](../CONTRIBUTING.md#changelog).
9. PR includes tests. allScreensTest when applicable, and unit tests
10. Avoid over complicating things. Keep it simple (KISS)!
11. PR contains only the expected change. Sometimes, the diff is showing changes that are already on `develop`. This is not good, submitter has to fix that up.
##### Check the commit
Commit message must be short, one line and valuable. "WIP" is not a good commit message. Commit message can contain issue number, starting with `#`. GitHub will add some link between the issue and such commit, which can be useful. It's possible to change a commit message at any time (may require a force push).
Commit messages can contain extra lines with more details, links, etc. But keep in mind that those lines are quite less visible than the first line.
Also commit history should be nice. Having commits like "Adding temporary code" then later "Removing temporary code" is not good. The branch has to be rebased and those commit have to be dropped.
PR merger could decide to squash and merge if commit history is not good.
Commit like "Code review fixes" is good when reviewing the PR, since new changes can be reviewed easily, but is less valuable when looking at git history. To avoid this, PR submitter should always push new commits after a review (no commit amend with force push), and when the PR is approved decide to interactive rebase the PR to improve the git history and reduce noise.
##### Check the substance
1. Test the changes!
2. Test the nominal case and the edge cases
3. Run the sanity test for critical PR
##### Make a dedicated meeting to review the PR
Sometimes a big PR can be hard to review. Setting up a call with the PR submitter can speed up the communication, rather than putting comments and questions in GitHub comments. It has the inconvenience of making the discussion non-public, consider including a summary of the main points of the "offline" conversation in the PR.
### What happen to the issue(s)?
The issue(s) should be referenced in the PR description using keywords like `Closes` of `Fixes` followed by the issue number.
Example:
> Closes #1
Note that you have to repeat the keyword in case of a list of issue
> Closes #1, Closes #2, etc.
When PR will be merged, such referenced issue will be automatically closed.
It is up to the person who has merged the PR to go to the (closed) issue(s) and to add a comment to inform in which version the issue fix will be available. Use the current version of `develop` branch.
> Closed in Element Android v1.x.y
### Merge conflict
It's up to the submitter to handle merge conflict. Sometimes, they can be fixed directly from GitHub, sometimes this is not possible. The branch can be rebased on `develop`, or the `develop` branch can be merged on the branch, it's up to the submitter to decide what is best.
Keep in mind that Github Actions are not run in case of conflict.
### When and who can merge PR
PR can be merged by the submitter, if and only if at least one approval from another developer is done. Approval from all people added as reviewer is also a good thing to have. Approval from design team may be mandatory, but is not sufficient to merge a PR.
PR can also be merged by the reviewer, to reduce the time the PR is open. But only if the PR is not in draft and the change are quite small, or behind a feature flag.
Dangerous PR should not be merged just before a release. Dangerous PR are PR that could break the app. Update of Realm library, rework in the chunk of Events management in the SDK, etc.
We prefer to merge such PR after a release so that it can be tested during several days by the team before behind included in a release candidate.
PR from bots will always be merged by the reviewer, right after approving the changes, or in case of critical changes, right after a release.
#### Merge type
Generally we use "Create a merge commit", which has the advantage to keep the branch visible.
If git history is noisy (code added, then removed, etc.), it's possible to use "Squash and merge". But the branch will not be visible anymore, a commit will be added on top of develop. Git commit message can (and probably must) be edited from the GitHub web app. It's better if the submitter do the work to cleanup the git history by using a git interactive rebase of their branch.
### Resolve conversation
Generally we do not close conversation added during PR review and update by clicking on "Resolve conversation"
If the submitter or the reviewer do so, it will more difficult for further readers to see again the content. They will have to open the conversation to see it again. it's a waste of time.
When remarks are handled, a small comment like "done" is enough, commit hash can also be added to the conversation.
Exception: for big PRs with lots of conversations, using "Resolve conversation" may help to see the remaining remarks.
Also "Resolve conversation" should probably be hit by the creator of the conversation.
## Responsibility
PR submitter is responsible of the incoming change. PR reviewers who approved the PR take a part of responsibility on the code which will land to develop, and then be used by our users, and the user of our forks.
That said, bug may still be merged on `develop`, this is still acceptable of course. In this case, please make sure an issue is created and correctly labelled. Ideally, such issues should be fixed before the next release candidate, i.e. with a higher priority. But as we release the application every 10 working days, it can be hard to fix every bugs. That's why PR should be fully tested and reviewed before being merge and we should never comment code review remark with "will be handled later", or similar comments.

22
docs/usefulLinks.md

@ -1,22 +0,0 @@ @@ -1,22 +0,0 @@
<!--- TOC -->
* [VersionCatalog](#versioncatalog)
* [Jetpack Compose](#jetpack-compose)
<!--- END -->
### VersionCatalog
https://docs.gradle.org/current/userguide/platforms.html
### Jetpack Compose
https://developer.android.com/jetpack/compose/mental-model
https://developer.android.com/jetpack/compose/libraries
https://developer.android.com/jetpack/compose/modifiers-list
https://android.googlesource.com/platform/frameworks/support/+/androidx-main/compose/docs/compose-api-guidelines.md#api-guidelines-for-jetpack-compose
Preview
https://alexzh.com/jetpack-compose-preview/
https://github.com/airbnb/Showkase
Loading…
Cancel
Save