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

Implemented new AddMultiTargetCompatibleSampleDocs target #141

Merged
merged 8 commits into from
Sep 29, 2023

Conversation

Arlodotexe
Copy link
Member

This PR:

  • Fixes [Sample App] Non-WASM supported components showing up in WASM Sample App #5.
  • Adds a new MSBuild inline task CheckMultiTarget to verify multi-target compatibility for the files provided to it.
  • Introduces a new target AddMultiTargetCompatibleSampleDocs which filters markdown files based on platform compatibility.
  • Includes error handling and logging in the MSBuild tasks to provide clearer feedback during build processes.
  • Excludes markdown files from obj and bin directories to prevent processing unnecessary files.
  • Has been tested on UWP and WASM.

Arlodotexe added a commit to CommunityToolkit/Windows that referenced this pull request Sep 19, 2023
Arlodotexe added a commit to CommunityToolkit/Windows that referenced this pull request Sep 19, 2023
Arlodotexe added a commit to CommunityToolkit/Windows that referenced this pull request Sep 19, 2023
@Arlodotexe Arlodotexe enabled auto-merge (squash) September 21, 2023 17:01
@michael-hawker
Copy link
Member

I realized I haven't opened a PR for the doc export script: https://github.com/CommunityToolkit/Tooling-Windows-Submodule/blob/llama/export-docs/Scripts/ExportDocs.ps1

One thing we'll have to think about is how we mark which platforms are supported within the docs and that script. Like for now for WASM it's easiest to remove the markdown files, but we may want to modify this approach to inject the supported platforms into the doc and possible just turn-off sample compilation for unsupported platforms or something.

What that be something we can add in the future with this approach or require other tooling?

@Arlodotexe
Copy link
Member Author

Arlodotexe commented Sep 25, 2023

One thing we'll have to think about is how we mark which platforms are supported within the docs and that script. Like for now for WASM it's easiest to remove the markdown files, but we may want to modify this approach to inject the supported platforms into the doc and possible just turn-off sample compilation for unsupported platforms or something.

The script you've linked is for modifying the documents before exporting for publishing on learn.microsoft.com. It replaces our custom ![SAMPLE_NAME] syntax with the content of the sample and updates all learn.microsoft.com links to be relative.

It sounds like you want to display which MultiTarget or TargetFramework a component is supported on from the learn.microsoft.com side of things for our users. Would this be as inline text, as metadata, or something else? This would be in addition to the information available on the "Frameworks" tab shown on nuget.org.

We'll need to also display this information in the header for the sample gallery. For display purposes, we should avoid doing this inline. That means we'll need the following:

  1. The DocumentationRenderer needs to identify what MultiTargets are supported for the component a given document is contained in, and display it in the header.
  2. The doc export script (the one linked, not yet in the repo) provided will need to pull from MultiTarget.props and inject it as front-matter, which can be consumed and rendered by docfx. We can extract/reuse existing code in GenerateMultiTargetAwareProjectReferenceProps.ps1 for this, but it's not yet clear how we'd display this on the learn.microsoft.com side of things.

Once we have these, we can display the supported MultiTarget or TargetFrameworks for a component within both the sample gallery and the published documentation.

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Seems to work well. Later we'll want to adjust the template/solution generation steps to not include the single-WASM head in the components that won't support it. Same for things like UWP in the future if we have something that is only built against WASDK.

@Arlodotexe Arlodotexe merged commit 1b6a0a6 into main Sep 29, 2023
8 checks passed
@delete-merged-branch delete-merged-branch bot deleted the fix/samples/multitarget-aware-sample-docs branch September 29, 2023 19:42
Arlodotexe added a commit to CommunityToolkit/Windows that referenced this pull request Oct 3, 2023
Arlodotexe added a commit to CommunityToolkit/Windows that referenced this pull request Oct 3, 2023
Arlodotexe added a commit to CommunityToolkit/Windows that referenced this pull request Oct 3, 2023
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.

[Sample App] Non-WASM supported components showing up in WASM Sample App
2 participants