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

docs: updating Codeowners #20512

Merged
merged 31 commits into from Apr 5, 2022
Merged

docs: updating Codeowners #20512

merged 31 commits into from Apr 5, 2022

Conversation

alyssawilk
Copy link
Contributor

@alyssawilk alyssawilk commented Mar 24, 2022

Fixing the tooling to pull the current maintainers list
Moving all extensions into CODEOWNERS.md and finding owners for all extensions.
go, go Envoy community!

Risk Level: n/a (docs / tooling)
Testing: fix format checks
Docs Changes: yes
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk
Copy link
Contributor Author

cc @envoyproxy/envoy-maintainers can folks call out "owned" extensions you'd be Ok owning?

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a ton for working on this. I'm just flushing out some initial thoughts.

CODEOWNERS Outdated
Comment on lines 192 to 193
/*/extensions/network/dns_resolver/cares @yanavlasov @UNOWNED
/*/extensions/network/dns_resolver/apple @yanavlasov @UNOWNED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can take these.

CODEOWNERS Outdated
Comment on lines 200 to 202
/*/extensions/filters/common/ext_authz @esmet @gsagula @UNOWNED
/*/extensions/filters/http/ext_authz @esmet @gsagula @UNOWNED
/*/extensions/filters/network/ext_authz @esmet @gsagula @UNOWNED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to own this personally, but calling out this is critical and we need an owner. I will work on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add @pradeepcrao to the owner list for ext_authz

CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@yanavlasov yanavlasov self-assigned this Mar 24, 2022
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk
Copy link
Contributor Author

Ok, I think I addressed all of the comments (but if you commented, PTAL and make sure I didn't mix things up)

CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated
Comment on lines 243 to 247
/*/extensions/tracers/zipkin @UNOWNED @UNOWNED
/*/extensions/tracers/dynamic_ot @UNOWNED @UNOWNED
/*/extensions/tracers/opencensus @UNOWNED @UNOWNED
/*/extensions/tracers/common @UNOWNED @UNOWNED
/*/extensions/tracers/common/ot @UNOWNED @UNOWNED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracing in general needs better ownership. Perhaps @lizan and @Shikugawa ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracing system is ok for me. I had some relatively large PRs before about tracing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also can help with it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try to help with the tracers. If we have the option can I get zipkin?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also help on tracers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @basvanbeek! Can you open a PR to add yourself?

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@wbpcode
Copy link
Member

wbpcode commented Mar 29, 2022

LGTM. And tracing system is ok for me. You can add me to the owner list. And cc @Shikugawa also can helps. And cc @suniltheta is interested in zipkin. Thanks. 😄

/*/extensions/tracers/zipkin @UNOWNED @UNOWNED
/*/extensions/tracers/dynamic_ot @UNOWNED @UNOWNED
/*/extensions/tracers/opencensus @UNOWNED @UNOWNED
/*/extensions/tracers/common @UNOWNED @UNOWNED
/*/extensions/tracers/common/ot @UNOWNED @UNOWNED

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
rgs1
rgs1 previously approved these changes Mar 29, 2022
Copy link
Member

@rgs1 rgs1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for my rgs1, thanks!

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
CODEOWNERS Outdated Show resolved Hide resolved
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@PiotrSikora
Copy link
Contributor

Moving all extensions into CODEOWNERS.md with @unowned to handle not-fully-owned extensions

Nit: @unowned seems to be a real account. Perhaps creating a team under @envoyproxy would make more sense than giving the codeowners rights to a stranger?

@alyssawilk
Copy link
Contributor Author

changing this file doesn't affect permissions, and I don't think it can cc them on reviews as they're not in org.

CODEOWNERS Outdated
# IP tagging
/*/extensions/filters/http/ip_tagging @rgs1 @UNOWNED
# mongo proxy
/*/extensions/filters/network/mongo_proxy @mythra @giantcroc @UNOWNED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do stamps on this after @Mythra and @giantcroc since I wrote the original code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG. I'll take ip tagging and formatting since @rgs1 has been so helpful to the comunity, and @daixiang0 had offered to help in general so I'll tag him as the second for cors, and we're good!

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link

@KfreeZ KfreeZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, I'd like to help with VCL

Copy link

@KfreeZ KfreeZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, I'd like to help with VCL (/contrib/vcl/), thanks

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@mattklein123
Copy link
Member

Oops needs main merge.

/wait

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay

@alyssawilk alyssawilk merged commit fa385b2 into envoyproxy:main Apr 5, 2022
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
Fixing the tooling to pull the current maintainers list
Moving all extensions into CODEOWNERS.md and finding owners for all extensions.
go, go Envoy community!

Risk Level: n/a (docs / tooling)
Testing: fix format checks
Docs Changes: yes
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk deleted the codeowners branch August 4, 2022 01:13
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 this pull request may close these issues.

None yet