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

Rename ElasticSearch instrumentation #248

Merged
merged 4 commits into from
Mar 17, 2022
Merged

Rename ElasticSearch instrumentation #248

merged 4 commits into from
Mar 17, 2022

Conversation

utpilla
Copy link
Contributor

@utpilla utpilla commented Mar 17, 2022

Towards #196

Changes

  • Remove the word "Contrib"

We still need to rename the folder names under src\ and test.

@utpilla utpilla requested a review from a team March 17, 2022 19:53
@utpilla
Copy link
Contributor Author

utpilla commented Mar 17, 2022

@ejsmith Could you please review these changes? We intend to remove the word "Contrib" completely from the contrib repo packages. Next steps would be to rename the src\ and test\ folders and release a package with the new name OpenTelemetry.Instrumentation.ElasticSearchClient on Nuget.

@codecov
Copy link

codecov bot commented Mar 17, 2022 •
edited
Loading

Codecov Report

Merging #248 (99a95f8) into main (af709ae) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #248   +/-   ##
=======================================
  Coverage   79.14%   79.14%           
=======================================
  Files          97       97           
  Lines        2498     2498           
=======================================
  Hits         1977     1977           
  Misses        521      521
Impacted Files Coverage Δ
...searchClient/ElasticsearchClientInstrumentation.cs 100.00% <ø> (ø)
...lient/ElasticsearchClientInstrumentationOptions.cs 100.00% <ø> (ø)
...ntation/ElasticsearchInstrumentationEventSource.cs 0.00% <ø> (ø)
.../ElasticsearchRequestPipelineDiagnosticListener.cs 82.35% <ø> (ø)
...ticsearchClient/TracerProviderBuilderExtensions.cs 87.50% <ø> (ø)

ejsmith
ejsmith reviewed Mar 17, 2022
View reviewed changes
...ion.ElasticsearchClient.Tests/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests.csproj
@@ -19,7 +19,7 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\OpenTelemetry.Contrib.Instrumentation.ElasticsearchClient\OpenTelemetry.Contrib.Instrumentation.ElasticsearchClient.csproj" />
<ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Contrib.Instrumentation.ElasticsearchClient\OpenTelemetry.Instrumentation.ElasticsearchClient.csproj" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this have Contrib removed as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking of renaming the folders in a separate PR to make it a little easier to review.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, as far as just changing the namespaces, this looks good to me.

ejsmith
ejsmith approved these changes Mar 17, 2022
View reviewed changes
Copy link
Contributor

@ejsmith ejsmith left a comment

Choose a reason for hiding this comment

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

LGTM

...ion.ElasticsearchClient.Tests/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests.csproj
@@ -19,7 +19,7 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\OpenTelemetry.Contrib.Instrumentation.ElasticsearchClient\OpenTelemetry.Contrib.Instrumentation.ElasticsearchClient.csproj" />
<ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Contrib.Instrumentation.ElasticsearchClient\OpenTelemetry.Instrumentation.ElasticsearchClient.csproj" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, as far as just changing the namespaces, this looks good to me.

@utpilla utpilla merged commit dff0556 into open-telemetry:main Mar 17, 2022
@utpilla utpilla mentioned this pull request Mar 17, 2022
Merged
swetharavichandrancisco pushed a commit to swetharavichandrancisco/opentelemetry-dotnet-contrib that referenced this pull request Apr 4, 2022
@utpilla @swetharavichandrancisco
Rename ElasticSearch instrumentation (open-telemetry#248)
a6173fb 
* Rename ElasticSearchInstrumentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cijothomas cijothomas cijothomas approved these changes

@ejsmith ejsmith ejsmith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@utpilla @ejsmith @cijothomas