Discussion: Internal - Pull Requests management

This is an attempt to lay out some ground rules in order to improve our handling of PRs.

Hopefully, this will avoid finding ourselves in the current situation introduced by styled-components PRs.

I believe this should be part of some guidelines documentation once discussed & approved.

Rules for merging PRs

Base rules

• Every person who merges a PR should have tested it by itself on at least one device.
• Every merged PR should have a related issue (merger can create one if needed).
• Never merge your own PRs.

Exceptions

• Doc: a PR that only changes documentation files (contributors, readme, ..)
• Typo: a PR that fix a typo in translations or in some variable naming
• QuickFix: a PR that fixes an obvious mistake in js logic causing a know issue (bad if condition, ..)
• Crash: a one-linish PR that fix a crash in master.

Additionally, for those cases, if the person submitting the PR is a maintainer, he can commit directly to master (or merge his own PR without approval).

Tests

At some point, we will need to start requiring tests for every significant change.

We still need to have more diversified tests samples available in order to ease the task on the pull requester.

Enforcements

• UI: If the PR introduces a new UI, or a change in the UI (styled-components, ..), the PR should be tested on both iOS & Android by the person who merges it.
• UI: The PR needs to have screenshots for both Android & iOS. (either the pull-requester or merger should provide them in the discussion)
• i18n: The PR have to be approved by at least one native speaker before merge can happen

Start by reading the issue discussion and its proposed rules for merging pull requests, including the testing, UI, screenshot, and i18n requirements. No file, test, or documentation entry point is named; done would require agreement on the guidelines and a clearly identified documentation location.