Skip to content

Replace the use of dynamic with Reflection to support AOT#270

Merged
iancooper merged 12 commits intomasterfrom
NetworkVisor-dev-removedynamic
Apr 7, 2025
Merged

Replace the use of dynamic with Reflection to support AOT#270
iancooper merged 12 commits intomasterfrom
NetworkVisor-dev-removedynamic

Conversation

@iancooper
Copy link
Copy Markdown
Member

This is a replacement for #269 due to merge issues

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper iancooper requested a review from Copilot April 4, 2025 08:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 54 out of 64 changed files in this pull request and generated 1 comment.

Files not reviewed (10)
  • Darker.sln: Language not supported
  • Darker.sln.DotSettings: Language not supported
  • Directory.Packages.props: Language not supported
  • SampleMauiTestApp/App.xaml: Language not supported
  • SampleMauiTestApp/AppShell.xaml: Language not supported
  • SampleMauiTestApp/MainPage.xaml: Language not supported
  • SampleMauiTestApp/Platforms/Android/AndroidManifest.xml: Language not supported
  • SampleMauiTestApp/Platforms/Android/Resources/values/colors.xml: Language not supported
  • SampleMauiTestApp/Platforms/MacCatalyst/Entitlements.plist: Language not supported
  • SampleMauiTestApp/Platforms/MacCatalyst/Info.plist: Language not supported

{
if (int.TryParse(numberEntry.Text, out int number))
{
;
Copy link

Copilot AI Apr 4, 2025

Choose a reason for hiding this comment

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

Remove the extraneous semicolon on this line; it appears to be a leftover statement.

Suggested change
;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removed the extra semicolon. Kinda embarrassed by this. :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iancooper Unfortunately, I don't have permission to push the change to the new branch. I will refork the Dark repository to see if that helps. It might be easier for you to make this change for me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let me see if I can build, get in the first version, and then you ought to be able to merge. Not quite sure why this repository is behaving this way. The branch protection looks sensible.

@iancooper iancooper added dependencies Pull requests that update a dependency file github_actions Pull requests that update GitHub Actions code .NET Pull requests that update .net code 3 - Done labels Apr 4, 2025
Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper
Copy link
Copy Markdown
Member Author

@SteveBush Moving your PR here. GitHub refused to allow me to resolve merge issues with updates to package dependencies. Resolving here.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper
Copy link
Copy Markdown
Member Author

@SteveBush I need to look at our build here so that we can get Maui working. It needs a Windows build, not a Linux build. I may look at both a Windows and Linux build, so that we can only build the Maui samples on the Windows build

@SteveBush
Copy link
Copy Markdown
Contributor

SteveBush commented Apr 4, 2025

@SteveBush I need to look at our build here so that we can get Maui working. It needs a Windows build, not a Linux build. I may look at both a Windows and Linux build, so that we can only build the Maui samples on the Windows build.

@iancooper This seems like a lot of work for you. I'm happy to either do the job or remove the MAUI test project as it served its purpose.

@iancooper
Copy link
Copy Markdown
Member Author

@SteveBush I'll time box it. If we have folks using it for Maui, then it makes some sense. In the worst case, I can move the sample to a separate repo. But, hell, I may learn something.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper iancooper requested a review from Copilot April 7, 2025 08:06
@iancooper iancooper merged commit 6352a42 into master Apr 7, 2025
6 of 7 checks passed
@iancooper iancooper deleted the NetworkVisor-dev-removedynamic branch April 7, 2025 08:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 56 out of 65 changed files in this pull request and generated 3 comments.

Files not reviewed (9)
  • Darker.sln: Language not supported
  • Darker.sln.DotSettings: Language not supported
  • Directory.Packages.props: Language not supported
  • SampleMauiTestApp/App.xaml: Language not supported
  • SampleMauiTestApp/AppShell.xaml: Language not supported
  • SampleMauiTestApp/MainPage.xaml: Language not supported
  • SampleMauiTestApp/Platforms/Android/AndroidManifest.xml: Language not supported
  • SampleMauiTestApp/Platforms/Android/Resources/values/colors.xml: Language not supported
  • SampleMauiTestApp/Platforms/MacCatalyst/Entitlements.plist: Language not supported

if (int.TryParse(numberEntry.Text, out int number))
{
;
var person = _queryProcessor.ExecuteAsync(new GetPersonNameQuery(number)).Result;
Copy link

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

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

Using .Result on an asynchronous operation may cause deadlocks in the UI thread. Consider making the event handler async and using await.

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +33
private void OnAllPeopleButtonClicked(object sender, EventArgs e)
{
var people = _queryProcessor.ExecuteAsync(new GetPeopleQuery()).Result;

Copy link

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

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

Blocking on asynchronous tasks using .Result in a UI event handler can lead to deadlocks. Refactor the code to use async/await for better responsiveness.

Suggested change
private void OnAllPeopleButtonClicked(object sender, EventArgs e)
{
var people = _queryProcessor.ExecuteAsync(new GetPeopleQuery()).Result;
private async void OnAllPeopleButtonClicked(object sender, EventArgs e)
{
var people = await _queryProcessor.ExecuteAsync(new GetPeopleQuery());

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,6 @@
namespace SampleMauiTestApp.Domain
{
public sealed class SomethingWentTerriblyWrongException : Exception
Copy link

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding standard exception constructors (e.g. parameterless, message, and inner exception overloads) to improve error reporting and usability.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Done dependencies Pull requests that update a dependency file github_actions Pull requests that update GitHub Actions code .NET Pull requests that update .net code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants