gitpoint / gitpoint/git-point

Discussion: Internal - Pull Requests management

Open
#625 9 comments 0 reactions 0 assignees View on GitHub
discussion
Dominant language
JavaScript
Stars
4.8k
Forks
771
PR merge metrics
No merged PRs in 30d

Description

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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.