docs: add PR workflow section in contribution guide (#7602)

**Motivation**

Document the PR workflow based on the last team
[discussion](https://github.com/ChainSafe/lodestar/discussions/7554#discussioncomment-12537466).

**Description**

- Explain different natures of PRs
- Explain steps how to open and manage those PRs. 

**Steps to test or reproduce**

- Run all tests

---------

Co-authored-by: Matthew Keil <me@matthewkeil.com>
Co-authored-by: Nico Flaig <nflaig@protonmail.com>
This commit is contained in:
Nazar Hussain
2025-03-26 19:10:21 +01:00
committed by GitHub
parent 706f45e815
commit 30652b83f0
2 changed files with 49 additions and 0 deletions

View File

@@ -209,6 +209,54 @@ for (const blockResult of blocksResult) {
}
```
## Managing and Opening Feature/Large PRs
To maintain code quality, improve collaboration, and ensure clarity in large or complex changes, we follow these guidelines when opening pull requests (PRs). Depending on the nature of the change, PRs fall into three categories:
### 1. Single, Complete PR
If the PR contains a self-contained and complete feature or bug fix that does not require major refactoring or cross-team discussions, then:
- Clearly explain the rationale and motivation behind the change in the PR description.
- Provide relevant context, including:
- Problem the PR is solving.
- Why the approach was chosen.
- Any alternatives considered.
- If the PR modifies critical code paths, add references to relevant issues, benchmarks, or related discussions.
- Ensure the PR adheres to our standard PR etiquette and commit message guidelines.
### 2. PR with Major Refactoring
If the PR involves significant code refactoring, structural changes, or fundamental modifications where team input is needed:
- Start a GitHub Discussion or Discord Thread before writing code.
- Outline the problem, your proposed approach, and any alternative solutions.
- Request feedback and build consensus with the team.
- Summarize the outcome and link the discussion in the PR description once consensus is reached.
- If changes affect multiple packages or require coordination with ongoing development, summarize any key decisions from the discussion in the PR description.
### 3. Large Feature or Multi-PR Implementation
If the PR introduces large-scale changes, affecting multiple areas of the codebase or requiring step-by-step integration:
- Document the feature first before opening any PR.
- Open a GitHub Discussion with a detailed technical proposal explaining the feature. This ideally will include some details like:
- Big picture explanation of how and why the feature will help or will change the codebase
- Rough outline of the classes and interfaces that will be implemented
- If a functional implementation is used, a brief description of what each function will do, possibly with a basic function signature if it is clear what will be needed
- Broad overview of how data will flow and integrate with the surrounding sub-systems
- Rough discussion of potential performance (cpu and memory) implications
- Share the document with the team and gather feedback before implementation.
- Create a feature branch to showcase the entire implementation. The idea will be to get this branch deployed on a feature group to test.
- First merge any refactor work necessary to get `unstable` prepared for the feature
- Create a second empty feature branch off of `unstable` that fork after the refactor work is merged. (Can also do this before the refactor work but then it will need to be rebased onto `unstable` after that refactor work is merged)
- Break down the implementation into smaller, manageable PRs that merge into the empty feature branch
- Each PR should focus on a specific part of the feature. This middle part of the review is focused on API, implementation overview and other high level pieces but will be relatively limited as the API discussion, analysis of the feature branch and full review on merge to `unstable` are the important steps.
- Link the design document in each PR description so reviewers can always refer to the full scope.
- Merge the smaller PRs into the feature branch until the complete feature is ready for a final merge into the `unstable` branch. This is the "formal review process" where several team members will likely get involved. Up to this point its mostly peer review. The merge to `unstable` is where details like naming, function signature, type definitions, etc will be scrutinized. This is also where metrics from the initial implementation branch will get detailed analysis (ideally this has already been happening along the way to make sure there are no performance regressions).
- Some corner cases can be observed while testing on feature group. If any such issue is identified which was not brainstormed earlier must be bring to attention of the team.
- If and when possible try to release big features under opt-in feature flags.
## Logging policy
### Logging Levels