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

Update CODEOWNERS file #3143

Merged
merged 6 commits into from
Sep 25, 2023
Merged

Update CODEOWNERS file #3143

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d44caee
Update CODEOWNERS file
GustavoCaso Sep 19, 2023
d003a2d
add all teams for all files
GustavoCaso Sep 19, 2023
bb74227
add profiling team
GustavoCaso Sep 19, 2023
96daed6
use ruby-guild team for the catch all CODEOWNERS file
GustavoCaso Sep 19, 2023
4ebb4ef
Move from apm-ruby to tracing-ruby
lloeki Sep 20, 2023
f893a1c
Add `sig` to CODEOWNERS
lloeki Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions .github/CODEOWNERS
View file
Open in desktop
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1,5 +1,42 @@
* @DataDog/apm-ruby
* @DataDog/ruby-guild

# Public Documentation
/docs/Compatibility.md @DataDog/documentation
/docs/GettingStarted.md @DataDog/documentation
/docs/GettingStarted.md @DataDog/documentation

# Library
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this file is missing sig ownership

Copy link
Contributor

Choose a reason for hiding this comment

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

Added!

lib/datadog/appsec/* @DataDog/asm-ruby
lib/datadog/appsec.rb @DataDog/asm-ruby
lib/datadog/tracing/* @DataDog/tracing-ruby
lib/datadog/tracing.rb @DataDog/tracing-ruby
lib/datadog/opentelemetry/* @DataDog/tracing-ruby
lib/datadog/opentelemetry.rb @DataDog/tracing-ruby
lib/datadog/opentracer/* @DataDog/tracing-ruby
lib/datadog/opentracer.rb @DataDog/tracing-ruby
lib-injection/* @DataDog/tracing-ruby
Copy link
Contributor

Choose a reason for hiding this comment

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

I am questioning this a bit: while initially developed by tracing automatic injection sounds like a feature that applies to everyone, not just tracing.

lib/datadog/profiling/* @DataDog/profiling-rb
lib/datadog/profiling.rb @DataDog/profiling-rb
ext/* @DataDog/profiling-rb
Copy link
Contributor

Choose a reason for hiding this comment

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

This question is probably outside the scope of this specific PR, but should we rename ext/ since it appears to be a profiling only folder?

Copy link
Member

Choose a reason for hiding this comment

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

We can't as ext/ is a standard for Ruby gems with native extensions: https://guides.rubygems.org/gems-with-extensions/

Copy link
Contributor

Choose a reason for hiding this comment

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

My rationale here is:

  • ext being a generic folder for extensions, it should be owned by the guild.
  • ext/ddtrace_profiling_loader and ext/ddtrace_profiling_native_extension should be owned by profiling-rb

Copy link
Contributor

@lloeki lloeki Sep 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That said, currently profiling is the sole user of libdatadog, but that may/should change, therefore I would like to encourage ownership to be split:

  • extension code pertaining to profiling should live in its own profiling-owned subdirectories
  • extension code pertaining to libdatadog in general should live in guild-owned subdirectories

e.g:

  • ddtrace_profiling_loader has no relationship to profiling at all, merely libdatadog. I suggest we rename it to libdatadog_loader and share ownership.
  • ddtrace_profiling_native_extension may or may not contain code that pertains only to libdatadog in general. If so, I'd suggest splitting it away in some way and share ownership of the common bits.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can't as ext/ is a standard for Ruby gems with native extensions

Note that is is mostly conventional, The directory being ext is not actually technically required, but I would definitely refrain from changing it to anything else!

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, currently profiling is the sole user of libdatadog, but that may/should change

That's a good point. I think that may change in the not too distant future.


# RBS signatures
sig/datadog/appsec/* @DataDog/asm-ruby
sig/datadog/appsec.rbs @DataDog/asm-ruby
sig/datadog/tracing/* @DataDog/tracing-ruby
sig/datadog/tracing.rbs @DataDog/tracing-ruby
sig/datadog/opentelemetry/* @DataDog/tracing-ruby
sig/datadog/opentelemetry.rbs @DataDog/tracing-ruby
sig/datadog/opentracer/* @DataDog/tracing-ruby
sig/datadog/opentracer.rbs @DataDog/tracing-ruby
sig/datadog/profiling/* @DataDog/profiling-rb
sig/datadog/profiling.rbs @DataDog/profiling-rb

# Specs
spec/datadog/appsec/* @DataDog/asm-ruby
spec/datadog/tracing/* @DataDog/tracing-ruby
spec/datadog/tracing_spec.rb @DataDog/tracing-ruby
spec/datadog/opentelemetry/* @DataDog/tracing-ruby
spec/datadog/opentelemetry_spec.rb @DataDog/tracing-ruby
spec/datadog/opentracer/* @DataDog/tracing-ruby
spec/datadog/opentracer.rb @DataDog/tracing-ruby
spec/datadog/profiling/* @DataDog/profiling-rb
spec/datadog/profiling_spec.rb @DataDog/profiling-rb
Loading