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

Revert adding Sentry to global usings #1487

Closed
jhartmann123 opened this issue Feb 16, 2022 · 5 comments · Fixed by #1490
Closed

Revert adding Sentry to global usings #1487

jhartmann123 opened this issue Feb 16, 2022 · 5 comments · Fixed by #1490
Assignees
Projects

Comments

@jhartmann123
Copy link

jhartmann123 commented Feb 16, 2022

The latest Release 3.14.0 comes with the following feature:

Add Sentry to global usings when ImplicitUsings is enabled (true) (#1398)

We do have implicit usings enabled, with the intention was that only the often used "standard"-namespaces are added there.
Sentry on the other hand is a namespace which we use exactly once in our ASP.NET Core API projects: when configuring Sentry on startup.

For me as an "end user" of this library it does make absolutely no sense to have Sentry in the global usings.

Main issue:
The main conflict here is Sentry.User. I guess a User is not a really uncommon name for a class. Now, with the latest update we have a lot of ambiguous references between Sentry.User and our Domain-User. As a workaround I'd have to explicitly reference our Domain-User in a ton of classes over multiple projects. Which I really don't want to do.

So I vote for removing Sentry from global usings again.

@github-actions github-actions bot added this to To do in Default Feb 16, 2022
@bruno-garcia
Copy link
Member

bruno-garcia commented Feb 16, 2022

User is definitely high chance of conflict. I suggest we take a pragmatic approach:

  1. asap: release a patch release with a opt-out for ImplicitUsings (e.g: <SentryImplicitUsings>false)
  2. Add to the release notes and a troubleshooting page entry to the docs on how to opt out
  3. Rename Sentry.User to SentryUser on the next major change. Do such renames on other types that are high risk of conflict

@davidroth
Copy link

IMO adding Sentry to global usings was a mistake. This library is used only in the composition root in 99% of the cases. The potential for real naming clashes (User) and similar types (i.e. Sentry.Span vs System.Span<T>) is real and not worth it IMO.

@SimonCropp
Copy link
Contributor

wouldnt this suffice as a opt-out ?

 <ItemGroup>
    <Using Remove="Sentry" />

@bruno-garcia
Copy link
Member

Good tip, TIL. Worth adding to: https://docs.sentry.io/platforms/dotnet/troubleshooting/#implicit-usings

@bruno-garcia
Copy link
Member

We discussed and agreed to revert this feature, until we release 4.0 that will include renaming some types prone to conflict.
Lets spin off a new issue to track the class renames and adding this feature back in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants