Skip to content

Fix logs exporter module name - #3951

Merged
jkwatson merged 5 commits into
open-telemetry:mainfrom
elevenzqx:fix-log-module-name
Dec 7, 2021
Merged

Fix logs exporter module name#3951
jkwatson merged 5 commits into
open-telemetry:mainfrom
elevenzqx:fix-log-module-name

Conversation

@elevenzqx

Copy link
Copy Markdown
Contributor

Fix logs exporter module name and description

@linux-foundation-easycla

Copy link
Copy Markdown

CLA Not Signed

@codecov

codecov Bot commented Dec 2, 2021

Copy link
Copy Markdown

Codecov Report

Merging #3951 (f5582a0) into main (51556cd) will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #3951      +/-   ##
============================================
+ Coverage     89.81%   89.90%   +0.08%     
- Complexity     4195     4208      +13     
============================================
  Files           500      503       +3     
  Lines         12728    12752      +24     
  Branches       1230     1232       +2     
============================================
+ Hits          11432    11465      +33     
+ Misses          903      896       -7     
+ Partials        393      391       -2     
Impacted Files Coverage Δ
...toconfigure/OpenTelemetrySdkAutoConfiguration.java 61.90% <0.00%> (-0.89%) ⬇️
...io/opentelemetry/sdk/logs/data/LogDataBuilder.java 100.00% <0.00%> (ø)
.../opentelemetry/sdk/logs/SdkLogEmitterProvider.java 100.00% <0.00%> (ø)
.../opentelemetry/sdk/internal/PrimitiveLongList.java 100.00% <0.00%> (ø)
...opentelemetry/api/metrics/GlobalMeterProvider.java 100.00% <0.00%> (ø)
...telemetry/sdk/metrics/SdkMeterProviderBuilder.java 96.66% <0.00%> (ø)
...sdk/metrics/internal/state/MetricStorageUtils.java 100.00% <0.00%> (ø)
.../metrics/internal/state/TemporalMetricStorage.java 100.00% <0.00%> (ø)
...etry/exporter/otlp/internal/okhttp/OkHttpUtil.java 100.00% <0.00%> (ø)
...lemetry/api/metrics/ObservableLongMeasurement.java 0.00% <0.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51556cd...f5582a0. Read the comment docs.

@linux-foundation-easycla

Copy link
Copy Markdown

CLA Not Signed

@jack-berg jack-berg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@breedx-splk breedx-splk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yikes. 😁 Thanks thanks.

import java.io.InputStream;

// Adapted from the protoc generated code for LogsServiceGrpc.
/** Adapted from the protoc generated code for LogsServiceGrpc. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This isn't really javadoc so a comment it more appropriate. Let's revert this change

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I realized my comment was in "pending status". 🤦🏽

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, but I think this style is aligned with other source codes in project;
In addition, Javadoc single-line comment can not pass spotlessJavaCheck, just like this:

/**
 * Adapted from the protoc generated code for LogsServiceGrpc.
 */

Do I need revert this change or keep it, @anuraaga @jkwatson

@anuraaga anuraaga Dec 7, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In addition, Javadoc single-line comment can not pass spotlessJavaCheck, just like this:

I don't follow this - spotless is currently passing right? Let's go ahead and revert this - this is an internal class so it doesn't need real javadoc, this is an implementation detail comment targeted at code authors, not docs for end users. We like to make that distinction where possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see, already revert

import java.io.InputStream;

// Adapted from the protoc generated code for LogsServiceGrpc.
/** Adapted from the protoc generated code for LogsServiceGrpc. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think @anuraaga had deliberately not made this a Javadoc comment. Anuraag, can you confirm?

@jkwatson
jkwatson merged commit 76ae8d2 into open-telemetry:main Dec 7, 2021
This was referenced Dec 19, 2021
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.

5 participants