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

doc: audit of docstrings for dev-friendliness & relevance #85

Merged
merged 15 commits into from
Feb 27, 2023
Merged

Conversation

emilyashley
Copy link
Contributor

@emilyashley emilyashley commented Feb 15, 2023

Which problem is this PR solving?

Short description of the changes

  • adds module docstrings for Distro
  • additional class and function docstrings
  • more type annotations for return expectations
  • additional tests on stdout for local vis exporter

How to verify that this has the expected result

  • reading the docstrings is helpful to you

@MikeGoldsmith MikeGoldsmith added the type: maintenance The necessary chores to keep the dust off. label Feb 24, 2023
@emilyashley emilyashley marked this pull request as ready for review February 25, 2023 16:50
@emilyashley emilyashley requested a review from a team February 25, 2023 16:50
src/honeycomb/opentelemetry/distro.py Outdated Show resolved Hide resolved
src/honeycomb/opentelemetry/distro.py Outdated Show resolved Hide resolved
@@ -14,19 +14,27 @@
OTEL_SERVICE_NAME
)
from grpc import ssl_channel_credentials
OTEL_SERVICE_VERSION = "OTEL_SERVICE_VERSION"

DEBUG = "DEBUG"
Copy link
Member

Choose a reason for hiding this comment

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

During sync review: let's put these ENV VAR constants and the credential ones near each other with a comment that they are ENV VAR name constants.

or a TraceIdRatio as 1/N.

Each of these samplers is ParentBased, meaning it respects
its parent span's sampling decision.
Copy link
Member

Choose a reason for hiding this comment

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

Huge fan! Even though we'll be writing the opposite here, soon. 😀

@emilyashley emilyashley merged commit 90fbf88 into main Feb 27, 2023
@emilyashley emilyashley deleted the emash-76 branch February 27, 2023 21:11
emilyashley added a commit that referenced this pull request Feb 27, 2023
<!--
Thank you for contributing to the project! 💜
Please see our [OSS process
document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#)
to get an idea of how we operate.
-->

## Which problem is this PR solving?

- Closes #31 

## Short description of the changes

built off of #85 , please review that one first! Only the last 3 commits
in this PR are finalizing turning on the linting in CI.
## How to verify that this has the expected result

In CircleCI, the linting step should run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance The necessary chores to keep the dust off.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dev] audit of docstrings for dev-friendliness & relevance
4 participants