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

Merge dev into main #677

Merged
merged 34 commits into from
Apr 7, 2023
Merged

Merge dev into main #677

merged 34 commits into from
Apr 7, 2023

Conversation

timsneath
Copy link
Contributor

@halildurmus Are we ready to review and potentially merge this? There is probably some back and forth between us, but I'd like to move forward if we can!

@halildurmus
Copy link
Member

As you mentioned here, we need to decide whether to keep IInspectable in this package or not. I'm inclined towards keeping it here.

If we're gonna keep it here, I'd like to propose some changes to both the IInspectable and IUnknown interfaces.

lib/src/com/iunknown.dart Outdated Show resolved Hide resolved
@timsneath
Copy link
Contributor Author

As you mentioned here, we need to decide whether to keep IInspectable in this package or not. I'm inclined towards keeping it here.

If we're gonna keep it here, I'd like to propose some changes to both the IInspectable and IUnknown interfaces.

My rationale for including it here is that it's a COM interface that is part of the Win32 metadata (namespace of Windows.Win32.System.WinRT). It's a peer to other Win32 APIs like RoInitialize and WindowsCreateString that sit at the boundary of WinRT. LMK if you have a different principle that you're applying.

lib/src/combase.dart Outdated Show resolved Hide resolved
@halildurmus
Copy link
Member

halildurmus commented Apr 7, 2023

I removed IInspectable extension methods and added them as top-level functions instead (inspired by How do I access and customize the IInspectable methods of a Windows Runtime class written in C++/WinRT?).

Copy link
Member

@halildurmus halildurmus left a comment

Choose a reason for hiding this comment

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

LGTM

@timsneath timsneath merged commit f9f0def into main Apr 7, 2023
4 checks passed
@timsneath timsneath deleted the dev branch April 7, 2023 23:27
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.

None yet

3 participants