Skip to content

Doc WinAppSDK's WinRT registration choice #2359

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

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

DrusTheAxe
Copy link
Member

@DrusTheAxe DrusTheAxe commented Apr 4, 2022

This is a polished form of the information in issue #2352 WinAppSdk is difficult to use with reg-free WinRT due to class/file naming, pairs
and particularly in this comment.

@DrusTheAxe DrusTheAxe added documentation Improvements or additions to documentation area-Infrastructure Build, test, source layout, package construction (TODO: move to Deployment, DeveloperTools) labels Apr 4, 2022
@DrusTheAxe DrusTheAxe added this to the 1.1 milestone Apr 4, 2022
@DrusTheAxe DrusTheAxe self-assigned this Apr 4, 2022
@ghost ghost added the needs-triage label Apr 4, 2022
@DrusTheAxe
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Comment on lines 34 to 36
While some might call this "non-ideal layout" (implying projections' manifest-free resolution is
ideal), that's not true. The manifest-free option is *simple*, but not necessarily best. As in
WinAppSDK's case.
Copy link
Contributor

Choose a reason for hiding this comment

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

Well it depends. It's non-ideal for some projection authors (me). It's very ideal for WinAppSdk for reasons below. Maybe consider striking this as the next paragraph does a good job at preparing the reader for The Big Why™️ after it.

Copy link
Member

Choose a reason for hiding this comment

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

Just delete and assert what we do and why. No need to defend it. If you are so motivated, you could write an appendix on how projection-based lookup doesn't meet our current needs w.r.t. naming of components.

Copy link
Member

@jonwis jonwis left a comment

Choose a reason for hiding this comment

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

This doc reads like a justification of the choice made, not a documentation. "We needed to accomplish X, our options were Q and Y and Z, option Z met the criteria for X."

Comment on lines 34 to 36
While some might call this "non-ideal layout" (implying projections' manifest-free resolution is
ideal), that's not true. The manifest-free option is *simple*, but not necessarily best. As in
WinAppSDK's case.
Copy link
Member

Choose a reason for hiding this comment

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

Just delete and assert what we do and why. No need to defend it. If you are so motivated, you could write an appendix on how projection-based lookup doesn't meet our current needs w.r.t. naming of components.

@riverar
Copy link
Contributor

riverar commented May 10, 2022

@DrusTheAxe I can open a separate issue on this if needed, but the embedded fusion manifest requirement appears to deviate from documented expectations, in that a manifest can be embedded as a resource or be on disk, as a fallback. The embedded-only approach that WAS has implemented is a pain for Rust that has no native tooling to embed a manifest.

Was this just deemed unnecessary at the time?

@DrusTheAxe
Copy link
Member Author

/azp run

@DrusTheAxe DrusTheAxe enabled auto-merge (squash) October 19, 2022 15:53
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DrusTheAxe DrusTheAxe merged commit 757d8c2 into main Oct 19, 2022
@DrusTheAxe DrusTheAxe deleted the user/drustheaxe/winrt-registration-doc branch October 19, 2022 17:06
DrusTheAxe added a commit that referenced this pull request Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure Build, test, source layout, package construction (TODO: move to Deployment, DeveloperTools) documentation Improvements or additions to documentation needs-triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants