-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
add cloud-artifacts as codeowners of /schemas/dbt #7406
Conversation
|
||
### ARTIFACTS | ||
|
||
/schemas/dbt @dbt-labs/cloud-artifacts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that is fine. We have PR reviews enforced so its not like an accidental commit can be pushed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long-term nit: this should be a CODENOTIFY
file instead, so we're not blocked on the Cloud Artifacts review?
https://about.sourcegraph.com/blog/a-different-way-to-think-about-code-ownership
|
||
### ARTIFACTS | ||
|
||
/schemas/dbt @dbt-labs/cloud-artifacts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that is fine. We have PR reviews enforced so its not like an accidental commit can be pushed
@aranke - I don't believe this will block merging on code review from Cloud Artifacts based on the behaviour I've been seeing for changes to files owned by existing codeowners defined. Here's a recent example that modified I'll create a test PR to confirm the behaviour once this is merged though! |
resolves N/A
Description
Checklist
changie new
to create a changelog entry