-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add IMuteIssuesService #5068
Add IMuteIssuesService #5068
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 potentially important changes
[PartCreationPolicy(CreationPolicy.Shared)] | ||
internal class MuteIssuesService : IMuteIssuesService | ||
{ | ||
private readonly IConfigurationProvider configurationProvider; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the same mistake in one of my PRs, but only realized it just now - I think we shouldn't use configuration provider directly, but instead use the cached configuration from IActiveSolutionBoundTracker for performance reasons
|
||
public async Task Mute(string issueKey, CancellationToken token) | ||
{ | ||
if (!configurationProvider.GetConfiguration().Mode.IsInAConnectedMode()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest adding throwifnotonbackgroundthread or an explicit switch to it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: Just throwing if on UI thread should be enough
Kudos, SonarCloud Quality Gate passed! |
8274e54
into
feature/mute-server-issues
Fixes #5059