Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strengthen reviewing controlling #799

Closed
wuhuizuo opened this issue Aug 27, 2024 · 1 comment · Fixed by #798
Closed

Strengthen reviewing controlling #799

wuhuizuo opened this issue Aug 27, 2024 · 1 comment · Fixed by #798

Comments

@wuhuizuo
Copy link

Strengthen reviewing controlling

An approval process for the change to the file path scope of global variables and configurations will be deployed.

Why

The current reviewers and approvers are too vague with organization level(https://github.com/tikv/community/blob/master/teams/tikv/team.json) and are not subdivided into specific repositories or areas, can not fine-grained control over changes in this repository. In order to make the review process more precise and standardized, we plan to deploy the review control plugin in the "tikv/rust-rocksdb" repository and delegate approval responsibilities to active "maintainers" and "committers" of this repository.

What's new for developer

  • The reviewing control framework will be migrated to the native "lgtm" and "approve" plug-ins, but reviewers’ habits do not need to change.
  • PR changes need to collect enough LGTM from reviewers or approvers that
  • All the changes need to be approved by approvers defined in OWNERS files. The approvers will be automatically recommended by bot on GitHub.

Ref:

Set up

  1. Now we use prow's OWNERS mechanism to control the approving for pull requests, here is the PR review flow.
  2. OWNERS files should be updated on time.

Role and Responsibility

  • For contributor
    • Assess the system's needs and requirements before making any configuration changes, considering factors such as performance, scalability, security, and compatibility.
    • Implement the configuration changes and verify that any changes made to the codebase or configuration maintain backward compatibility.
    • Conduct testing to verify the effects of configuration changes, checking for any adverse impact on system behavior, performance, or compatibility.
    • In case of any issues or unexpected outcomes resulting from the configuration changes, contributors actively troubleshoot and work towards resolving them.
  • For approver
    • Thoroughly review all proposed configuration change pull requests. This involves examining the details of the proposed modifications, such as the purpose, impact, and potential risks associated with the configuration change.
    • Evaluate the impact these changes may have on the system or any related components. This includes considering potential risks, such as performance impact, security vulnerabilities, or conflicts with existing configurations.
    • Validate that the proposed configuration changes in the pull requests have undergone appropriate testing.
    • Grant approval for the PR.
@wuhuizuo
Copy link
Author

/type enhancement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@wuhuizuo and others