-
Notifications
You must be signed in to change notification settings - Fork 485
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
fix: add safekeeper team to pgxn codeowners #7170
Conversation
e.g., https://github.com/neondatabase/neon/commits/main/pgxn/neon/walproposer.c is part of the safekeeper |
@arssher wdyt? does it make sense to require compute team review for changes to the C code that we link into postgres, or would you like your team to be able to do these changes unilaterally? The other side effect of this change would be that your team would get auto-assigned as reviewers for pgxn code that is unrelated to the safekeeper client code, and be responsible for either reviewing or delegating that. |
2706 tests run: 2576 passed, 0 failed, 130 skipped (full report)Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
28acbda at 2024-03-20T15:35:28.728Z :recycle: |
We definitely do want to be able to make them unilaterally; if this is not the case, I just manually ask for the review.
True, but there is no easy ideal solution. Even without it generally in my experience this codeowners thing quite frequently assigns unsuitable reviewers, but previous point (being able to merge without unrelated team reviews) is more important. |
We should also add @neondatabase/safekeepers to /libs/postgres_ffi/. In fact likely this PR is caused by |
Signed-off-by: Alex Chi Z <chi@neon.tech>
Problem
pgxn/
also contains WAL proposer code, so modifications to this directory should be able to be approved by the safekeeper team.Summary of changes
Checklist before requesting a review
Checklist before merging