Code Review Guidance
To continuously improve the quality of Apache SeaTunnel code, we have compiled this code review guide.
We expect reviewers and committers to follow this guidance consistently, especially for documentation, e2e coverage, and compatibility-sensitive changes.
Approval policy for PRs targeting dev
GitHub branch protection on dev requires one approval before merge. The same baseline applies to all modules, including core modules such as seatunnel-api and seatunnel-engine:
- One committer approval
- Passing automated bot checks, such as CI, code style, and license validation
A reviewer may ask for a second committer review when they consider a change risky. This is based on the reviewer's judgment, not on which modules the PR touches. Examples include:
- Checkpoint or serialization format changes
- Public API changes in
seatunnel-api - Features proposed through a STIP
- Incompatible changes
If a second committer review has been requested, do not merge the PR until that review is given, even if GitHub reports that the required review check has passed.
For PRs that touch core modules, committers are encouraged to wait at least 24 hours after approval before merging, so that committers in other time zones have a chance to review the change or request a second review. This is a recommendation, not a merge requirement. The core modules are:
seatunnel-apiseatunnel-engine/seatunnel-engine-coreseatunnel-engine/seatunnel-engine-serverseatunnel-engine/seatunnel-engine-clientseatunnel-engine/seatunnel-engine-commonseatunnel-engine/seatunnel-engine-serializerseatunnel-engine/seatunnel-engine-storage
General review checklist
- Check whether the PR title follows project conventions and accurately describes the change.
- Check whether bug fixes link the related issue, and whether major changes link a design document.
- Check whether documentation has been added or updated when needed, and whether the documentation is correct. A good example is PR #4590.
- Check whether e2e tests should be added, and whether the e2e coverage is correct. Review both function coverage and result validation, including supported data types, source and target column alignment, row counts, and row-level data correctness. A good example is the ClickHouse e2e case.
- Check whether the change introduces incompatible behavior, especially parameter changes. If an incompatible change is really necessary, it should be discussed on the mailing list first.
- Check CI results, license updates, and other release-readiness signals.
Component-specific review checklist
- For enumerator changes, check whether split snapshot and restore are correct, and whether the split allocation strategy remains stable.
- For reader changes, check split snapshot handling, checkpoint lock scope, and all end conditions in
pollNext. - For sink changes, check whether two-phase commit logic in
XXXCommitter(if any) is still correct. - For writer changes, check data flush frequency, flush interval, memory usage, batch size, and other resource-sensitive behavior.
- After the functional checks above pass, review the code style. Code style should support readability without weakening functional correctness. For additional style references, see ShardingSphere's code conduct guide.