Tooling-Windows-Submodule icon indicating copy to clipboard operation
Tooling-Windows-Submodule copied to clipboard

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

Open michael-hawker opened this issue 2 years ago • 5 comments

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 avatar Jul 25 '23 21:07 michael-hawker

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 https://github.com/CommunityToolkit/Windows/pull/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 avatar Jul 25 '23 22:07 michael-hawker

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.

michael-hawker avatar Jul 25 '23 23:07 michael-hawker

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

Arlodotexe avatar Jan 09 '24 18:01 Arlodotexe

@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?

michael-hawker avatar Jan 09 '24 22:01 michael-hawker

@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.

Arlodotexe avatar Jun 20 '24 23:06 Arlodotexe