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

Fixed an issue where all documents were include regardless of MultiTarget support #237

Merged
merged 7 commits into from
Oct 3, 2023

Conversation

Arlodotexe
Copy link
Member

@Arlodotexe Arlodotexe commented Sep 19, 2023

PR Type

Fixes an issue where all documents were include regardless of MultiTarget support.

  • Bugfix

What is the current behavior?

Before, the tooling would blindly pull in all markdown files and let the SampleGen source generator sort it out for creating the Document Registry.

This is in contrast to the Sample Registry, which inherits its MultiTarget-aware characteristics from MSBuild by discovering only in projects that are referenced at build time.

What is the new behavior?

With these changes, markdown files are preprocessed with the new Target. Files found in components that don't <MultiTarget> the platform being built are filtered out before being added as AdditionalFiles. Then, it's picked up by the SampleGen tooling to create the Document Registry.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • Tested code with current supported SDKs
  • New component
    • Documentation has been added
    • Sample in sample app has been added
    • Analyzers are passing for documentation and samples
    • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
  • Tests for the changes have been added (if applicable)
  • Header has been added to all new source files
  • Contains NO breaking changes

Other information

@Arlodotexe Arlodotexe added the bug Something isn't working label Sep 19, 2023
@Arlodotexe Arlodotexe self-assigned this Sep 19, 2023
@Arlodotexe Arlodotexe changed the title Fixed an issue where all components were include regardless of MultiTarget support Fixed an issue where all documents were include regardless of MultiTarget support Sep 20, 2023
@michael-hawker
Copy link
Member

I may have to try another machine, I haven't been able to get past unoplatform/Uno.Wasm.Bootstrap#776 building this at the moment.

@michael-hawker
Copy link
Member

@Arlodotexe tested this against the latest changes in the CommunityToolkit/Tooling-Windows-Submodule#141 PR that this is on top of, this PR needs to be updated though, but otherwise, good to go.

@Arlodotexe Arlodotexe enabled auto-merge (rebase) October 3, 2023 16:08
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.

Tested this earlier with the submodule changes, so should be all good, thanks!

@Arlodotexe Arlodotexe merged commit 82eb9a9 into main Oct 3, 2023
9 checks passed
@delete-merged-branch delete-merged-branch bot deleted the fix/samples/multitarget-aware-sample-docs branch October 3, 2023 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sample app 🖼️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants