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

Change AttributeValue sequences from optional to nonoptional #1855

Merged
merged 7 commits into from
May 21, 2021

Conversation

eddyleelin
Copy link
Contributor

@eddyleelin eddyleelin commented May 15, 2021

Description

Per attribute specifications, None values should be discouraged in AttributeValues, but Python cannot actually prevent None values. Thus, the new changes will ensure that mypy users will be alerted when trying to add None to an AttributeValue array, without altering the current code that may continue to accept None.

Fixes issue #1738

Type of change

This is a simple bug fix that will help mypy users conform with specifications.

How Has This Been Tested?

Running the following test suites returned a clean report:

tox -e test-core-api
tox -e mypy

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated

New changes will ensure that mypy users will be alerted when trying to add None to an AttributeValue array, without altering the current code that may continue to accept None.
@eddyleelin eddyleelin requested review from a team, aabmass and lzchen and removed request for a team May 15, 2021 02:01
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Change looks good, please update the branch and add a Changed header to the changelog.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Please resolve the changelog conflict and we can get this PR merged, thanks!

@codeboten codeboten merged commit 2098c56 into open-telemetry:main May 21, 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.

4 participants