Skip to content

Conversation

NN---
Copy link
Contributor

@NN--- NN--- commented Jan 30, 2025

Description

Python json.dumps has optional indent parameter which is not reflected in the API.
This is not a breaking change, only the type is updated.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

No test is needed, it is only typing change.

Does This PR Require a Contrib Repo Change?

7

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@NN--- NN--- requested a review from a team as a code owner January 30, 2025 15:25
Copy link

linux-foundation-easycla bot commented Jan 30, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@aabmass
Copy link
Member

aabmass commented Jan 30, 2025

Python json.dumps has optional indent parameter which is not reflected in the API.
This is not a breaking change, only the type is updated.

@NN--- the change looks good but can you add some context? Also why would you want to pass None?

@NN---
Copy link
Contributor Author

NN--- commented Jan 30, 2025

I want to write json to file with each span on a single line.

@NN---
Copy link
Contributor Author

NN--- commented Feb 22, 2025

It turns out that Optional is deprecated in favor of type | None.
Resolving conflicts and updating the code.

@NN--- NN--- requested a review from lzchen February 23, 2025 18:01
@lzchen
Copy link
Contributor

lzchen commented Feb 24, 2025

@NN---

It turns out that Optional is deprecated in favor of type | None.

Were you planning on making this change?

@NN---
Copy link
Contributor Author

NN--- commented Feb 24, 2025

I think making this in a separate PR.
Don't have time for it now.

@emdneto
Copy link
Member

emdneto commented Feb 26, 2025

It turns out that Optional is deprecated in favor of type | None. Resolving conflicts and updating the code.

This syntax wont work for py38 and will break a lot of tests in contrib, so better to leave for another PR

@xrmx xrmx merged commit c44df70 into open-telemetry:main Feb 27, 2025
384 checks passed
@github-project-automation github-project-automation bot moved this from Easy to review / merge / close to Done in @xrmx's Python PR digest Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants