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

migrate dotnet/templating to dotnet/sdk #4353

Closed
23 of 26 tasks
vlada-shubina opened this issue Feb 3, 2022 · 3 comments
Closed
23 of 26 tasks

migrate dotnet/templating to dotnet/sdk #4353

vlada-shubina opened this issue Feb 3, 2022 · 3 comments
Assignees
Labels
7.0 Cost:XL Work that requires one engineer more than 4 weeks Epic Groups multiple user stories. Can be grouped under a theme. Priority:2 Work that is important, but not critical for the release triaged The issue was evaluated by the triage team, placed on correct area, next action defined.
Milestone

Comments

@vlada-shubina
Copy link
Member

vlada-shubina commented Feb 3, 2022

Background

Previous releases of the SDK have been made more difficult due to the separation between templating (where dotnet new is implemented and the console and classlib templates are located), the SDK (where dotnet new is hooked into the overall command structure), and the installer repo (where these disparate components are stitched together). Versioning mismatches, template content mismatches, and automated code flow blockages/insertion failures are all categories of errors we experienced that slowed down the release cadence.

To mitigate these points of failure, some or all of the templating repository could be merged into the SDK repository. What this means is still under discussion. The high-level goals are:

  • Mitigate the risk of mismatched System.CommandLine packages, since the package is not yet stable
  • Mitigate the risk of incompatible .NET Runtime versions, since with the repos separate there's some orchestration here that often must happen in sync
  • Mitigate the risk of incomplete testing due to the templating testing environment not being an accurate representation of the 'full' SDK environment

2 proposals

  • Merge the repos in their entirety, and
  • Merge the dotnet new command and the templates, leaving the core engine in the templating repo

The costs and benefits of these two approaches are still being determined by the team.


Decision is to go with second approach:

  • migrate the following projects to dotnet/sdk (preserving history); remove duplicate files - use them from SDK directly
    • Microsoft.TemplateEngine.Cli
    • dotnet-new3
    • *.UnitTests, *.IntegrationTests.
    • template_feed ?
  • migrate unit and integration tests; however run then in separate job in existing pipeline in parallel with other jobs; integration tests should be run over sdk
    • use test framework directly from SDK (remove copied files).
    • migrate Microsoft.TemplateEngine.CLI.IntegrationTests to other assemblies (dotnet-new3.IntegrationTests, Microsoft.TemplateEngine.IDE.IntegrationTests Microsoft.TemplateEngine.Edge.IntegrationTests) - will be done after Enable AUTHORING UX approval tests creation  #3868
    • add missing tests to Microsoft.TemplateEngine.IDE.IntegrationTests or Microsoft.TemplateEngine.Edge.IntegrationTests - will be done after Enable AUTHORING UX approval tests creation  #3868
  • migrate template_feed to dotnet\sdk
  • migrate issues related to dotnet new to dotnet\sdk with correct label - won't do
  • move dotnet-new3 to dotnet\sdk;
  • create filtered solution for dotnet new related projects in dotnet\sdk; ensure that projects and unit tests are usable from VS.
  • update codeowners file
  • update analyzer rules in both repos
  • remove code from dotnet/templating, update readme - Removed projects moved to dotnet/sdk #5079
    • update readme and docs in dotnet/templating
    • update readme in dotnet/sdk
      • main readme
      • readme for templates
      • readme for dotnet new
      • introduce different labels: Area-Templates - for template config, Area-dotnet new or similar for dotnet new.
  • ensure that templates are still flowing to dotnet/installer fine, adapt dependencies and subscriptions where needed (dotnet/templating, dotnet/sdk, dotnet/installer)
  • ensure that tests are still run in parallel in dotnet/sdk - not 100% sure about this
  • refactor after merge - in progress @vlada-shubina
    • Reporter
    • telemetry
    • post action callbacks
    • tests refactoring (remove duplicate code)
  • package and publish TestHelper instead of copied code in dotnet/sdk

Justification

Engineering impact

  1. Mitigate the risk of mismatched packages and runtime versions
  2. Mitigate the risk of incomplete testing due to the templating testing environment not being an accurate representation of the 'full' SDK environment
@bekir-ozturk bekir-ozturk added the triaged The issue was evaluated by the triage team, placed on correct area, next action defined. label Feb 7, 2022
@bekir-ozturk bekir-ozturk added this to the .NET 7.0 milestone Feb 7, 2022
@vlada-shubina vlada-shubina added the User Story A single user-facing feature. Can be grouped under an epic. label Mar 30, 2022
@vlada-shubina
Copy link
Member Author

Related: dotnet/sdk#23385

@donJoseLuis donJoseLuis added 7.0 Priority:2 Work that is important, but not critical for the release Cost:XL Work that requires one engineer more than 4 weeks parent:1465581 and removed parent:1465581 labels Apr 5, 2022
@vlada-shubina vlada-shubina modified the milestones: .NET 7.0, June 2022 May 4, 2022
@baronfel
Copy link
Member

baronfel commented Jun 2, 2022

One more task that I think should be added to this list: post-migration, remove the subscription to dotnet/runtime. I think we just re-added it, but it shouldn't be necessary after the CLI is moved to dotnet/sdk, right?

@vlada-shubina
Copy link
Member Author

One more task that I think should be added to this list: post-migration, remove the subscription to dotnet/runtime. I think we just re-added it, but it shouldn't be necessary after the CLI is moved to dotnet/sdk, right?

This is orthogonal task. Dependency on runtime ensures tests are run on latest runtime. We added it after bad regression that we had in .NET 5.0 that went unnoticed for couple of previews. The issue was in core library.

Now that we moved main testing effort to sdk repo, which anyway ensures latest runtime, it is safer to remove dependency, as the tests in sdk may catch the issue. However, the risk of regression cannot be fully eliminated, it may happen if there is no test. I suggest we try.

@donJoseLuis donJoseLuis added test and removed test labels Dec 13, 2022
@donJoseLuis donJoseLuis added Epic Groups multiple user stories. Can be grouped under a theme. Iteration:2022August and removed User Story A single user-facing feature. Can be grouped under an epic. labels Dec 23, 2022
@donJoseLuis donJoseLuis modified the milestones: August 2022, 7.0RC1 Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.0 Cost:XL Work that requires one engineer more than 4 weeks Epic Groups multiple user stories. Can be grouped under a theme. Priority:2 Work that is important, but not critical for the release triaged The issue was evaluated by the triage team, placed on correct area, next action defined.
Projects
None yet
Development

No branches or pull requests

5 participants