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

Hide WinRT - Part II #290

Merged
merged 12 commits into from
Sep 20, 2023
Merged

Hide WinRT - Part II #290

merged 12 commits into from
Sep 20, 2023

Conversation

CarlosNihelton
Copy link
Contributor

And season finale. With the following set of changes we are ready to work on the mock client API.

There was a bug in the DLL's vcxproj: The DefineConstants property is a C# thing. With VC++, we need to add a ClCompile item with the PreprocessorDefinitions flag set. That's the way we do in WSL launcher end-to-end tests (https://github.com/ubuntu/WSL/blob/main/meta/UbuntuPreview/src/DistroLauncher/Preprocessor.props).

Closes UDENG-1380

A bit more pedantic header inclusion
No coroutines
Now placed inside the service.
UserInfo related was moved to the context and to the header.
no more coroutines
DefineConstants is a C# property.
For C++. it must be PreprocessorDefinitions under a ClCompile item.
Also, the logic was twisted
@CarlosNihelton CarlosNihelton force-pushed the hide-winrt-part-II branch 2 times, most recently from 92ec533 to e6c31d5 Compare September 19, 2023 03:06
Removes coroutine and WinRT related headers,
Updates stubs methods
Removes no longer relevant test for user id size
No test should isntantiate production Context
Token must not be empty
@CarlosNihelton CarlosNihelton marked this pull request as ready for review September 19, 2023 12:38
Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell left a comment

Choose a reason for hiding this comment

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

Very nice. Thanks for splitting this PR and the previous one.

I have a couple comments (the godbolt one you can ignore haha).

msix/storeapi/StoreApi.cpp Outdated Show resolved Hide resolved
storeapi/test/stubs.hpp Outdated Show resolved Hide resolved
storeapi/base/impl/StoreContext.cpp Show resolved Hide resolved
Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell left a comment

Choose a reason for hiding this comment

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

🚂 🚋 🚋 🚋

@CarlosNihelton CarlosNihelton merged commit 9f60462 into main Sep 20, 2023
30 checks passed
@CarlosNihelton CarlosNihelton deleted the hide-winrt-part-II branch September 20, 2023 11:58
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.

2 participants