Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.56%. Comparing base (28bc599) to head (6ae26c7).
Report is 1103 commits behind head on dev.

@@            Coverage Diff             @@
##              dev    #9431      +/-   ##
==========================================
+ Coverage   89.82%   90.56%   +0.73%     
==========================================
  Files         412      405       -7     
  Lines       11878    12701     +823     
  Branches     2364     2459      +95     
==========================================
+ Hits        10670    11503     +833     
+ Misses        681      638      -43     
- Partials      527      560      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I understand forwarding the new event to the old one. But doesn't removing public from the old events break existing code where people rely on the interface to subscribe these two events when using IDialogService as the type of the injected service?

[Inject]
IDialogService DialogService { get; set; }

I understand forwarding the new event to the old one. But doesn't removing public from the old events break existing code where people rely on the interface to subscribe these two events when using IDialogService as the type of the injected service?

Interface members in C# are public by default. it's a contract.
Explicit visibility should only be defined when you're making a default implementation or logic that shouldn't be visible (again, this is used for the default implementation).
Before C# 8.0, it wasn't possible to set the public keyword, and this is the exception you'd get:

Adding the public keyword to members without default implementation doesn't do any harm, I just find it incorrect language usage since it was intended solely for dealing with default implementations, and here we have only one such member.

Understood. Thanks for explaining ;)