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 configuration change approval #371

Merged

Conversation

wuhuizuo
Copy link

@wuhuizuo wuhuizuo commented Apr 19, 2024

Ref pingcap/tiflash#8966

What is changed and how it works?

Issue Number: Close #xxx

What's Changed:


Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Release note

None

@CLAassistant
Copy link

CLAassistant commented Apr 19, 2024

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot ti-chi-bot bot added the size/S label Apr 19, 2024
@wuhuizuo wuhuizuo force-pushed the chore/add-prow-owners-files branch from d8ec8cb to ae7d505 Compare April 19, 2024 02:43
@wuhuizuo wuhuizuo marked this pull request as draft April 19, 2024 02:44
@wuhuizuo wuhuizuo force-pushed the chore/add-prow-owners-files branch from ae7d505 to 9803aa1 Compare April 19, 2024 05:56
@wuhuizuo
Copy link
Author

/run-unit-test

@wuhuizuo
Copy link
Author

/test unit-test

Copy link

ti-chi-bot bot commented Apr 19, 2024

@wuhuizuo: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-unit-test

Use /test all to run all jobs.

In response to this:

/test unit-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wuhuizuo
Copy link
Author

/test pull-unit-test

OWNERS_ALIASES Outdated Show resolved Hide resolved
@wuhuizuo wuhuizuo marked this pull request as ready for review May 26, 2024 06:00
@wuhuizuo wuhuizuo force-pushed the chore/add-prow-owners-files branch from 2e5047f to 835322c Compare May 26, 2024 06:07
@ti-chi-bot ti-chi-bot bot added size/XL and removed size/S labels May 26, 2024
@ti-chi-bot ti-chi-bot bot added size/S and removed size/XL labels May 26, 2024
@wuhuizuo
Copy link
Author

@ti-chi-bot ti-chi-bot bot requested a review from CalvinNeo May 26, 2024 06:09
Copy link

ti-chi-bot bot commented May 26, 2024

@wuhuizuo: GitHub didn't allow me to request PR reviews from the following users: yudongusa, zanmato1984, easonn7.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @easonn7 @yudongusa @CalvinNeo @zanmato1984

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

OWNERS_ALIASES Outdated Show resolved Hide resolved
Signed-off-by: wuhuizuo <[email protected]>
@wuhuizuo wuhuizuo force-pushed the chore/add-prow-owners-files branch from aa126ef to 293239b Compare May 26, 2024 06:11
@wuhuizuo
Copy link
Author

I do not known why the build-check CI task failed.

@CalvinNeo
Copy link
Member

CalvinNeo commented May 28, 2024

@wuhuizuo If we merged tikv/master, which introduces some new configurations, do we also need to approve them here?

@CalvinNeo
Copy link
Member

I do not known why the build-check CI task failed.

Because the free space provided by github is not enough

@wuhuizuo
Copy link
Author

@wuhuizuo If we merged tikv/master, which introduces some new configurations, do we also need to approve them here?

@CalvinNeo Sorry for the late reply.

No, here we just controlled the config.rs in proxy_components/proxy_server/src folder, unless we also merged the OWNERS of the upstream repository, if so it will need the approvals from the tikv approvers.

Of course, there are additional benefits at this time: knowing important changes and understanding the related impacts with experts. The focus at this time is to understand the upstream changes rather than the approval process (which happens less frequently).

@wuhuizuo
Copy link
Author

wuhuizuo commented Jun 3, 2024

/hold

it should be merged by @wuhuizuo

@wuhuizuo
Copy link
Author

wuhuizuo commented Jun 3, 2024

Ref: ti-community-infra/configs#1067

@wuhuizuo wuhuizuo force-pushed the chore/add-prow-owners-files branch from f84c291 to 2aa44c5 Compare June 3, 2024 11:46
@CalvinNeo CalvinNeo merged commit f9322ae into pingcap:raftstore-proxy Jun 4, 2024
3 of 4 checks passed
@wuhuizuo wuhuizuo deleted the chore/add-prow-owners-files branch June 4, 2024 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants