Skip to content

Notification on outdated Rider package #2244

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
merged 5 commits into from
Jan 24, 2022
Merged

Conversation

van800
Copy link
Member

@van800 van800 commented Jan 14, 2022

@van800 van800 added this to the Rider 2022.1 milestone Jan 14, 2022
@van800 van800 requested a review from citizenmatt January 14, 2022 13:55
@van800 van800 self-assigned this Jan 14, 2022
Copy link
Member

@citizenmatt citizenmatt left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few comments for things I wasn't sure about.

Comment on lines 118 to 127
var notification = RiderNotification.Create(NotificationSeverity.WARNING,
"JetBrains Rider package in Unity is missing.",
"Make sure JetBrains Rider package is installed in Unity Package Manager."
);
myShellLocks.ExecuteOrQueueEx(lt,
"RiderPackageUpdateAvailabilityChecker.ShowNotificationIfNeeded",
() =>
{
myNotificationPopupHost.ShowNotification(lt, notification);
});
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be simplified to just UserNotifications.CreateNotiification. AFAICT, this will both create and show the notification with one API.

@van800 van800 merged commit cb0b115 into net221 Jan 24, 2022
@van800 van800 deleted the net221-notification-package branch January 24, 2022 14:45
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