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

Merged ISpanContext into ITraceContext #2668

Merged
merged 4 commits into from
Sep 26, 2023

Conversation

jamescrosswell
Copy link
Collaborator

@jamescrosswell jamescrosswell commented Sep 26, 2023

This interface contained no properties or methods... and only impleneted ITraceContext so is entirelyreplacable by ITraceContext.

Migration Notes

Methods and properties now available on ITraceContext (although likely soon to be merged into ISpanData - so we'll probably tell users to look for the methods and properties on ISpanData instead).

@jamescrosswell jamescrosswell added this to the 4.0.0 milestone Sep 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2023

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Merged ISpanContext into ITraceContext ([#2668](https://github.com/getsentry/sentry-dotnet/pull/2668))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 78325cc

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Just a note on release notes/migration guide.

CHANGELOG.md Outdated
@@ -14,6 +14,10 @@ without native/platform specific bindings and SDKs. See [this ticket for more de

- Drop .NET 6 Mobile in favor of .NET 7 ([#2624](https://github.com/getsentry/sentry-dotnet/pull/2604))

API Changes:

- Removed ISpanContext ([#2668](https://github.com/getsentry/sentry-dotnet/pull/2668))
Copy link
Member

Choose a reason for hiding this comment

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

Does this affect the end user in any way? Doesn't seem so, but worth being clear here.

At the end of the day we'll need a migration guide so users know what steps users need to take in order to upgrade the SDK. So when we say a type was removed, they might expect: "Ok what do I need to do to replace it?". In this case it's possibly just "This type had no documented use and if somehow you used it, it can be replaced with ITraceContext" or something like that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I'll try to paraphrase that in the changelog for now... and put a bit more detail in the PR notes that we can use when preparing the migration guide proper.

@jamescrosswell jamescrosswell changed the title Feat/4.0.0 remove ISpanContext Merged ISpanContext into ITraceContext Sep 26, 2023
@jamescrosswell jamescrosswell merged commit 300f248 into feat/4.0.0 Sep 26, 2023
5 of 6 checks passed
@jamescrosswell jamescrosswell deleted the feat/4.0.0-remove-ispancontext branch September 26, 2023 21:39
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.

2 participants