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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove depends_on from Core Configuration option #3085

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

GustavoCaso
Copy link
Member

@GustavoCaso GustavoCaso commented Aug 28, 2023

What does this PR do?

Based on internal conversations, we agreed that depends_on is a feature from the past and that is no longer used. It has been replaced with the different components. Because of that we agreed that it was good to remove it 馃敟

Motivation:

We are cleaning up the codebase from options we no longer use.

I was able to remove the following:

  • The depends_on option from the configuration DSL
  • Configuration::DependencyResolver.
  • Configuration::OptionDefinitionSet. It was inherited from Hash, and the only functionality added was to organize the options in order using the depends_on attribute. I replaced it with a normal Hash.
  • Configuration::OptionSet. It was inherited from Hash, but we were not extending the class with any particular functionality. I replaced it with a normal Hash.

Additional Notes:

How to test the change?

CI must be 馃煝

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@github-actions github-actions bot added core Involves Datadog core libraries integrations Involves tracing integrations tracing labels Aug 28, 2023
@GustavoCaso GustavoCaso changed the title Remove depends_on configuration option Remove depends_on from Core Configuration option Aug 28, 2023
@GustavoCaso GustavoCaso force-pushed the core-configuration-remove-depends-on-option branch 2 times, most recently from 7b4c1b8 to af9ee8b Compare August 28, 2023 16:07
@GustavoCaso GustavoCaso force-pushed the core-configuration-remove-depends-on-option branch from af9ee8b to 5f0804e Compare August 28, 2023 16:18
@GustavoCaso GustavoCaso marked this pull request as ready for review August 28, 2023 16:29
@GustavoCaso GustavoCaso requested a review from a team August 28, 2023 16:29
Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

Let's make sure to include in this in our changelog.

Because this feature hasn't been functional to its advertised intent for quite some time, I'm 馃憤 to remove it in its current form.

@GustavoCaso GustavoCaso merged commit bae2327 into master Aug 29, 2023
202 of 203 checks passed
@GustavoCaso GustavoCaso deleted the core-configuration-remove-depends-on-option branch August 29, 2023 08:29
@github-actions github-actions bot added this to the 1.15.0 milestone Aug 29, 2023
@ivoanjo
Copy link
Member

ivoanjo commented Aug 30, 2023

Nice, hooray for less complexity! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants