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

Add template for creating new experiments #76

Merged
merged 42 commits into from
May 10, 2022
Merged

Conversation

mrlacey
Copy link
Contributor

@mrlacey mrlacey commented Apr 14, 2022

Draft only - for tracking progress

For #4 (Template Infrastructure)

Still to do:

  • Update to reflect changes in test projects
  • Add doc(s) files
  • Confirm what functionality (if any) should be in the generated classes
  • Confirm what functionality (if any) should be in the generated samples
  • Confirm what "working examples and helpful comments" are needed
  • Need a naming convention for Sample pages (and the directory--or directories--they are in)
  • Should the documentation file be visible inside the solution?
  • Finally: double-check the generated solutions all work as expected

current issues:

  • SourceGenerators issues when running the UWP sample head
  • WASM sample head doesn't include the shell

@mrlacey mrlacey mentioned this pull request Apr 14, 2022
@mrlacey
Copy link
Contributor Author

mrlacey commented Apr 14, 2022

How I'm (manually) testing this:

Generate a new project also called CanvasLayout but in a different folder. The diff the two "CanvasLayout" folders and verify the only differences are:

  • Project Guids
  • No functionality in the new src\CanvasLayout.cs file
  • Only one sample page in samples\CanvasLayout.Sample\
  • Different ports used in the WASM head
  • Minor white space differences

Also

  • Tests run in the newly generated version
  • Sample apps all run in the newly generated version

Then, generate another new experiment but with a different name and compare it with the previously generated CanvasLayout version and confirm the only differences are:

  • Project Guids
  • Project names (there should be no results in a search for "CanvsaLayout" in the version with a different name.)
  • Different ports used in the WASM head

Adding automated testing for this isn't going to be easy and is unlikely to be useful in the long term.

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.

Looking good, @Arlodotexe any feedback?

template/README.md Outdated Show resolved Hide resolved
Comment on lines 52 to 53
[ToolkitSample(id: nameof(SamplePage), "Simple Options", ToolkitSampleCategory.Controls, ToolkitSampleSubcategory.Layout, description: "A sample page for showing how to do simple options.")]
public sealed partial class SamplePage : Page
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the ProjectTemplateSamplePage here? That gets auto-replaced right? Would be easier to accidentally prevent multiple experiments having a SamplePage and leading to confusion later?

Or do we want to do like ProjectTemplateFirstSamplePage? And rename the namespace ProjectTemplate.Sample.FirstSample and folder to FirstSample to be consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would leave use with a lot of different experiments all creating files in a PT.S.FirstSample namespace which might look weird when they're put together ina a collective sample.

Do different sample pages have to be in separate directories? Do they need to be in directories at all--can't they just be in teh root of this project?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can just leave them in the root of the project without having folders for each. Or we don't have to create separate namespaces. The ToolkitSample attribute is going to expect that the id: nameof(SamplePage) is unique across all samples, so we don't want to have this be generically named and introduce the possibility of collisions.

@michael-hawker
Copy link
Member

Since the template project should be its own stand-alone project. It should be buildable on its own still, eh?

If so, in this PR, we should swap out our 'experiment' job in the CI YAML to build the template solution here as well. That should help us ensure that we don't break things if we change the template or the props files, right?

Or maybe we can invoke the template command itself to create a dummy experiment in that job and try and build that? Maybe that'd be a better "integration" type test for the templates?

Thoughts?

@mrlacey
Copy link
Contributor Author

mrlacey commented Apr 27, 2022

Since the template project should be its own stand-alone project. It should be buildable on its own still, eh?

If so, in this PR, we should swap out our 'experiment' job in the CI YAML to build the template solution here as well. That should help us ensure that we don't break things if we change the template or the props files, right?

Good idea

Or maybe we can invoke the template command itself to create a dummy experiment in that job and try and build that? Maybe that'd be a better "integration" type test for the templates?

I'm not sure how (or if) this would be done but it's a nice idea.

@michael-hawker
Copy link
Member

Or maybe we can invoke the template command itself to create a dummy experiment in that job and try and build that? Maybe that'd be a better "integration" type test for the templates?

I'm not sure how (or if) this would be done but it's a nice idea.

I'm thinking we can just add the two dotnet commands to install the template and invoke the template with some 'DummyTest' name within the existing yml file here:

- name: MSBuild
working-directory: ./labs/CanvasLayout
run: msbuild.exe CanvasLayout.sln /restore -p:Configuration=Release

And then we'd just build it with that known path/name instead of CanvasLayout one here.

I think that should be pretty simple.

@mrlacey mrlacey marked this pull request as ready for review May 9, 2022 12:37
@mrlacey
Copy link
Contributor Author

mrlacey commented May 9, 2022

I have moved this out of draft as I think it's ready.
Currently failing due to changes introduced by @Arlodotexe which I assume are in preparation for some other changes being made. (I assume for #30 - but unconfirmed.)
Once those changes are in this should be good to go.

@Arlodotexe
Copy link
Member

Hey @mrlacey, adding a second experiment uncovered an important issue with the sample and documentation registry systems and @michael-hawker asked me to investigate and fix. I'm getting things back in order - will be finished soon!

@mrlacey
Copy link
Contributor Author

mrlacey commented May 9, 2022

Hey @mrlacey, adding a second experiment uncovered an important issue with the sample and documentation registry systems and @michael-hawker asked me to investigate and fix. I'm getting things back in order - will be finished soon!

This is the issue I've been wanting there to me multiple experiments for.

@Arlodotexe
Copy link
Member

All finished here, we should have no problems with multiple experiments now.

@Arlodotexe
Copy link
Member

Rest of the code looks good from my end, just waiting on @michael-hawker to give the OK 🙂

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.

Looks good, I created #102 using this initially, so worked well. Will merge this and then test @Arlodotexe's fixes rebasing #102 and seeing how it works out.

@michael-hawker michael-hawker merged commit e3fcfa4 into main May 10, 2022
@delete-merged-branch delete-merged-branch bot deleted the mrlacey/labtemplate branch May 10, 2022 08:07
@michael-hawker michael-hawker mentioned this pull request May 10, 2022
11 tasks
Martin1994 pushed a commit to Martin1994/Labs-Windows that referenced this pull request Sep 2, 2023
…labtemplate

Add template for creating new experiments
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.

3 participants