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.

Sign up for GitHub

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

Jump to bottom

codeowners: Add field-engineering team #138241

Merged
merged 1 commit into from
Jan 7, 2025
Merged

codeowners: Add field-engineering team #138241

merged 1 commit into from
Jan 7, 2025

Conversation

BramGruneir
Copy link
Member

This PR adds the Field Engineering team so we can get alerts when our upcoming tests fail and what not.

Also, I alphabetized the TEAMS.yaml files and the owners.go list of teams because it makes it easier to read.
I can skip the second commit if this is not desired.

Also, why are there two TEAMS.yaml if they are identical?

@BramGruneir BramGruneir requested a review from a team as a code owner January 3, 2025 20:37
@BramGruneir BramGruneir requested review from nameisbhaskar and vidit-bhat and removed request for a team January 3, 2025 20:37
@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Jan 3, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@msbutler msbutler requested review from srosenberg and a team January 3, 2025 20:42
@srosenberg
Copy link
Member

srosenberg commented Jan 3, 2025 •
edited
Loading

Also, I alphabetized the TEAMS.yaml files and the owners.go list of teams because it makes it easier to read. I can skip the second commit if this is not desired.

Rules are matched bottom up, so I'm afraid alphabetizing may have broken something. I'd revert the second commit.

Also, why are there two TEAMS.yaml if they are identical?

We only care about the top-level one; the other one (under pkg/internal) is generated by bazel (see pkg/internal/team/BUILD.bazel).

@rickystewart
Copy link
Collaborator

rickystewart commented Jan 3, 2025 •
edited
Loading

is generated by bazel (see pkg/internal/team/BUILD.bazel).

Drive-by: There doesn't seem to be a reason for this extra TEAMS.yaml to a) be checked in or b) exist at all. You could add it to .gitignore, or better yet augment the test to allow loading the file from //:TEAMS.yaml, and prevent any further confusion.

ETA: it seems it's copied into this directory to be used in embedsrcs, so maybe not copying it at all is not an option, although I believe you could use a symlink. I agree having two identical copies of the same file in the repo doesn't make a great deal of sense.

@BramGruneir
Copy link
Member Author

So I'll split this in two. First, I'll just cut off the 2nd commit and then I'll try to address the weirdness of the 2nd copy when I have some time in a new PR.

@BramGruneir
Copy link
Member Author

This change now just adds FE. PTAL

@BramGruneir
codeowners: add field-engineering team
951d04a 
Adding field engineering in preparation of owning the code we are contributing.

Epic: none

Release note: None
@BramGruneir
Copy link
Member Author

BramGruneir commented Jan 7, 2025 via email

@craig
Copy link
Contributor

craig bot commented Jan 7, 2025

Build succeeded:

@craig craig bot merged commit a1c2b66 into cockroachdb:master Jan 7, 2025
21 of 22 checks passed
@BramGruneir BramGruneir deleted the add-field-eng-team branch January 7, 2025 15:25
@BramGruneir BramGruneir mentioned this pull request Jan 7, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srosenberg srosenberg srosenberg approved these changes

@nameisbhaskar nameisbhaskar Awaiting requested review from nameisbhaskar nameisbhaskar is a code owner automatically assigned from cockroachdb/test-eng

@vidit-bhat vidit-bhat Awaiting requested review from vidit-bhat vidit-bhat is a code owner automatically assigned from cockroachdb/test-eng

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BramGruneir @cockroach-teamcity @srosenberg @rickystewart