Skip to content
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

fix: remove all build warnings #1334

Merged

Conversation

JohnTasler
Copy link
Collaborator

@JohnTasler JohnTasler commented Feb 2, 2025

This cleans up all warnings that are generated when building Wpf.Ui.sln.

Pull request type

  • Build related changes

What is the current behavior?

Issue Numbers: #1332 and #1333

What is the new behavior?

No warnings are generated when building.

Other information

This PR closes #1332. Also closes #1333.

@github-actions github-actions bot added controls Changes to the appearance or logic of custom controls. PR Pull request dotnet release labels Feb 2, 2025
@github-actions github-actions bot added themes Topic is related to managing themes navigation Changes to navigation related controls. gallery WPF UI Gallery tray titlebar Titlebar updates labels Feb 3, 2025
@JohnTasler JohnTasler changed the title fix: remove all non-analyzer warnings fix: remove all build warnings Feb 3, 2025
@JohnTasler
Copy link
Collaborator Author

A couple of notes:

  1. On the TitleBar control, the analyzer said that the return value for Header and TrailingContent properties should be a object? rather than object. My concern is that since these are public properties, it might cause a breaking change for someone.
  2. The documentation for warning CA1060 says "Do not suppress a warning from this rule.". However, I did exactly that. I can think of some other ways, but they seem to be a lot of effort that may not be worth it. The easiest way would be to make the current static classes namespaces instead of classes, and then in those namespaces, to rename the class to NativeMethods. For example:
namespace Wpf.Ui.Interop.Shell32;

/// <summary>
/// The Windows UI provides users with access to a wide variety of objects necessary to run applications and manage the operating system.
/// </summary>
internal static class NativeMethods
{

This would require several code changes at all the call sites. However, these could be reduced using this syntax at the top of each file using the interop methods.

using Shell32 = Wpf.Ui.Interop.Shell32.NativeMethods;

Let me know how you want to proceed. Disable the warning as I have done here, or go through and fix all of them.

@pomianowski pomianowski self-assigned this Feb 4, 2025
@pomianowski pomianowski merged commit c6adc9a into lepoco:main Feb 4, 2025
2 checks passed
@pomianowski
Copy link
Member

Hey @JohnTasler, thanks for another PR and fixes to the library. Great that you also updated the packages. Changing object? to object was probably an accident, this value could potentially be null i think, so as a regression I would change it in 4.0.1 to object?.

As for the interop warning, I would think about it in another PR, I created a separate issue where it can be discussed #1338

@JohnTasler JohnTasler deleted the jt-remove-non-analyzer-build-warnings branch February 4, 2025 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controls Changes to the appearance or logic of custom controls. dotnet gallery WPF UI Gallery navigation Changes to navigation related controls. PR Pull request release themes Topic is related to managing themes titlebar Titlebar updates tray
Projects
None yet
2 participants