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

Adds the WinUI 2/3 Build script from the main repository #114

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

michael-hawker
Copy link
Member

as an additional job to test changes to tooling against our main code-base.

This will help us catch more issues with the build when we make tooling changes. Since before here (since we have no components) we weren't building the all-up sample app when making changes to it. This would have caught a number of issues we only found when trying to update our other repos to the latest tooling.

Probably needs #113 to succeed, so will be a good test?

@michael-hawker
Copy link
Member Author

michael-hawker commented Jul 25, 2023

Ah, this failed with:

         D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\LayoutTransformControlSample.xaml.cs(19,29): error TKSMPL0005: Cannot generate sample pane option with name ScaleX as the provided name is already defined as a member in the attached class [D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\LayoutTransformControl.Samples.csproj]
         D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\LayoutTransformControlSample.xaml.cs(19,29): error TKSMPL0005: Cannot generate sample pane option with name ScaleY as the provided name is already defined as a member in the attached class [D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\LayoutTransformControl.Samples.csproj]
         D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\CommunityToolkit.Tooling.SampleGen\CommunityToolkit.Tooling.SampleGen.ToolkitSampleOptionGenerator\LayoutTransformControlExperiment.Samples.LayoutTransformControlSample.Property.ScaleX.g.cs(9,23): error CS0114: 'LayoutTransformControlSample.ScaleX' hides inherited member 'View.ScaleX'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword. [D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\LayoutTransformControl.Samples.csproj]
         D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\CommunityToolkit.Tooling.SampleGen\CommunityToolkit.Tooling.SampleGen.ToolkitSampleOptionGenerator\LayoutTransformControlExperiment.Samples.LayoutTransformControlSample.Property.ScaleY.g.cs(9,23): error CS0114: 'LayoutTransformControlSample.ScaleY' hides inherited member 'View.ScaleY'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword. [D:\a\Tooling-Windows-Submodule\Tooling-Windows-Submodule\components\LayoutTransformControl\samples\LayoutTransformControl.Samples.csproj]

Which is being fixed in CommunityToolkit/Windows#160

So at least shows, we can catch more with this. Imagine if we add a matrix to include Labs-Windows repo for this as well that we'll hit the issue we are in the tests too?

@michael-hawker michael-hawker force-pushed the llama/build-main-repo branch from 4d9c244 to d7fa6e3 Compare July 25, 2023 23:15
@michael-hawker
Copy link
Member Author

Updated off latest tooling, and Windows main should be building with latest tooling, so those pipelines should build now.

Expect Labs-Windows to fail due to dependency/testing conflict with Segmented Control usage of the Framework Extension Ancestor.

@Arlodotexe
Copy link
Member

@michael-hawker Think we've got our processes nailed down here, we want to just close this for now?

@michael-hawker
Copy link
Member Author

@michael-hawker Think we've got our processes nailed down here, we want to just close this for now?

I mean this I guess is more of a proposal for a change in process, where we can just have a single PR in the tooling repo and see it vetted across the main repo, instead of having to open a PR there temporarily pointing to a branch here, merge a PR here, then go back update a PR in the main repo with the correct pointer for the submodule, and then merge that in there.

Instead, we could try just having the CI run the main stuff for a change here, see that it's vetted, and then have a simple PR after to just validate the final update to the main repo after (which we should have high confidence to pass, as it would have done the litmus test here).

Thoughts?

@Arlodotexe
Copy link
Member

Arlodotexe commented Jun 20, 2024

@michael-hawker Think we've got our processes nailed down here, we want to just close this for now?

I mean this I guess is more of a proposal for a change in process, where we can just have a single PR in the tooling repo and see it vetted across the main repo, instead of having to open a PR there temporarily pointing to a branch here, merge a PR here, then go back update a PR in the main repo with the correct pointer for the submodule, and then merge that in there.

Instead, we could try just having the CI run the main stuff for a change here, see that it's vetted, and then have a simple PR after to just validate the final update to the main repo after (which we should have high confidence to pass, as it would have done the litmus test here).

Thoughts?

I think the process we've been doing makes sense for a submodule and plays out like publishing and consuming a prerelease nuget package, just with a submodule pointer instead of actually publishing a prerelease package and consuming it in a gallery repo/project. Trying to validate a dependent within one of its dependencies, even an integral one, seems to work against the way updates normally flow from library publisher to library consumer.

For the purposes of a pre-emptive litmus test, we can build locally or observe the CI in whichever repo we're updating. We don't always update the Windows and Labs-Windows repos in parallel, for example. Labs often lags slightly behind, which allows us to prioritize shipping fixes and enhancements in the main repository.

We might have more repos than just Windows and Labs-Windows using this Tooling submodule in the future, before we get a chance to create proper nuget packages and SDKs for all the code here. DataTable is the first that comes to mind, but there's several libraries that could one day be consolidated or upgraded, such as the IntelligentAPIs, ColorCode-Universal, Lottie-Windows, and more.

All that in mind, I propose keeping our current process and closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants