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

[ad] rename adService to ad #1832

Merged
merged 9 commits into from
Dec 19, 2024
Merged

Conversation

Sozhan308
Copy link
Contributor

@Sozhan308 Sozhan308 commented Dec 12, 2024

Changes

Renames adservice to ad.

This is part of the ongoing renaming for standarizing the component names

Merge Requirements

For new features contributions, please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs
  • Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

Copy link

linux-foundation-easycla bot commented Dec 12, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot added the helm-update-required Requires an update to the Helm chart when released label Dec 12, 2024
@puckpuck puckpuck added the docs-update-required Requires documentation update label Dec 12, 2024
@puckpuck
Copy link
Contributor

Can you add a Changelog entry to this?

@Sozhan308 Sozhan308 marked this pull request as ready for review December 13, 2024 16:23
@Sozhan308 Sozhan308 requested a review from a team as a code owner December 13, 2024 16:23
@Sozhan308
Copy link
Contributor Author

Sozhan308 commented Dec 13, 2024

Can you add a Changelog entry to this?

I have updated the changelog

@Sozhan308 Sozhan308 changed the title [adService] rename adService to ad [ad] rename adService to ad Dec 14, 2024
@puckpuck
Copy link
Contributor

I updated this PR, to update the environment variable AD_SERVICE_* -> AD_* and the feature flag names.

After the changes, I tested it, and Jaeger displayed traces as expected.

@Sozhan308
Copy link
Contributor Author

Sozhan308 commented Dec 14, 2024

I updated this PR, to update the environment variable AD_SERVICE_* -> AD_* and the feature flag names.

After the changes, I tested it, and Jaeger displayed traces as expected.

Thank you for updating and testing the changes, @puckpuck. Being new to this project, it took me some time to set it up locally to test the changes. I would also like to learn more about the project and explore opportunities to contribute to other topics. Would it be okay if I reach out to you, @puckpuck, to learn more about the project and its ongoing discussions?

src/ad/gradlew.bat Show resolved Hide resolved
kubernetes/opentelemetry-demo.yaml Outdated Show resolved Hide resolved
src/ad/src/main/java/oteldemo/AdService.java Outdated Show resolved Hide resolved
src/ad/src/main/java/oteldemo/AdService.java Outdated Show resolved Hide resolved
Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

Added some nits but LGTM

src/ad/gradlew.bat Outdated Show resolved Hide resolved
@puckpuck puckpuck merged commit d6efe7f into open-telemetry:main Dec 19, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-update-required Requires documentation update helm-update-required Requires an update to the Helm chart when released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants