Skip to content

Enable nullability in Glyph classes and BehaviorService#9341

Merged
dreddy-work merged 3 commits intodotnet:mainfrom
halgab:glyph-nullable
Jun 26, 2023
Merged

Enable nullability in Glyph classes and BehaviorService#9341
dreddy-work merged 3 commits intodotnet:mainfrom
halgab:glyph-nullable

Conversation

@halgab
Copy link
Contributor

@halgab halgab commented Jun 22, 2023

Proposed changes

  • Enable nullability in Glyph, GlyphCollection, ToolStripItemGlyph and BehaviorService
Microsoft Reviewers: Open in CodeFlow

@ghost ghost assigned halgab Jun 22, 2023
@ghost ghost added area-NRT draft draft PR labels Jun 22, 2023
@halgab halgab marked this pull request as ready for review June 22, 2023 13:42
@halgab halgab requested a review from a team as a code owner June 22, 2023 13:42
@ghost ghost removed the draft draft PR label Jun 22, 2023
IUIService uiService = (IUIService)_serviceProvider.GetService(typeof(IUIService));
if (uiService is not null)
IUIService? uiService = _serviceProvider.GetService<IUIService>();
IWin32Window? hwnd = uiService?.GetDialogOwnerWindow();
Copy link
Member

Choose a reason for hiding this comment

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

hwnd - > hWnd

Copy link
Member

Choose a reason for hiding this comment

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

technically, existing code is more optimized. hwnd won't be declared if uiService is null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to keep it short and optimized with pattern matching to address your comment

Behavior? behavior = GetAppropriateBehavior(_hitTestedGlyph);

if (behavior is not null)
{
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if this if block on line 564 can be optimized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reading it again, nothing obvious comes to mind unfortunately

@dreddy-work dreddy-work added the waiting-author-feedback The team requires more information from the author label Jun 22, 2023
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Jun 22, 2023
@halgab halgab requested a review from dreddy-work June 24, 2023 21:41
@dreddy-work dreddy-work merged commit 244b71a into dotnet:main Jun 26, 2023
@ghost ghost added this to the 8.0 Preview7 milestone Jun 26, 2023
@halgab halgab deleted the glyph-nullable branch June 26, 2023 19:23
@ghost ghost locked as resolved and limited conversation to collaborators Jul 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants