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

Set .NET used by our tools to net6.0 + minor updates to .sln files #4937

Merged
merged 21 commits into from
Dec 13, 2022

Conversation

konrad-jamrozik
Copy link
Contributor

@konrad-jamrozik konrad-jamrozik commented Dec 10, 2022

This PR addresses issues #4888, #4934 and #4935 for the following tools and other projects, by migrating their target frameworks to net6.0, and ensuring that their ci.yml sets DotNetCoreVersion: 6.0.403:

  • version-guard;
  • CreateRuleFabricBot and its tests;
  • code-owners-parser and its tests. This required disabling execution of get-codeowners.ps1 script as part of the test suite. Follow-up work to re-enable it: Re-enable code-owners-parser get-codeowners.ps1 test script after migration to net6.0 is done #4938;
  • notification-configuration;
  • identity-resolution;
  • generator (i.e. Stress.Generator) and its tests;
  • Azure.ClientSdk.Analyzers and its tests;
  • SwaggerApiParser. This only required setting ci.yml to point to .NET 6, as it was already targeting net6.0;
  • snippet-generator and its tests;
  • tools\agent-time-extractor\AgentTimeExtractor\AgentTimeExtractor.csproj;
  • src\dotnet\APIView\TestLibrary\TestLibrary.csproj, which was using netcoreapp2.1 which is out of support since August 2021, but the APIView pipeline and the pipeline seems to be failing due to unrelated reasons, so usage of netcoreapp2.1 seems to be non-blocking. Nevertheless, I updated it to net6.0 anyway;
  • Azure.Sdk.Tools.HttpFaultInjector;
  • Azure.Sdk.Tools.HttpFaultInjector.StorageBlobsSample;
  • Azure.Sdk.Tools.HttpFaultInjector.HttpClientSample;
  • Sample projects of TestProxy, but not the TestProxy itself, as it was already on net6.0, albeit it also targets net5.0, which I left unchanged;
  • PixelServer, which required adding an import to Program.cs. Note I couldn't find a corresponding ci.yml or pipeline for it.

Tools not updated to net6.0 in this PR

<TargetFramework> occurrences not migrated as part of this PR, and why:

  • Any tools targetting netstandard as it is compatible with .NET 6.0;
  • tools\pipeline-generator\Azure.Sdk.Tools.PipelineGenerator\Azure.Sdk.Tools.PipelineGenerator.csproj, because it is done in PR Update PipelineGenerator target framework from netcoreapp3.1 to net6.0 #4915
  • tools\check-enforcer\Azure.Sdk.Tools.CheckEnforcer\Azure.Sdk.Tools.CheckEnforcer.csproj, as it uses archetype-sdk-tool-azure-function.yml archetype, which doesn't support the DotNetCoreVersion override. CheckEnforcer currently targets netcoreapp3.1;
  • tools\github-issues\Azure.Sdk.Tools.GitHubIssues\Azure.Sdk.Tools.GitHubIssues.csproj, for the same reason as check-enforcer. GitHubIssues currently targets netcoreapp3.1.
  • tools\webhook-router\Azure.Sdk.Tools.WebhookRouter\Azure.Sdk.Tools.WebhookRouter.csproj, for the same reasons as check-enforcer. WebhookRouter currently target netcoreapp3.1.

Additional remarks

For a list of all PRs doing migration to net6.0, see this comment:

As a secondary mini-refactoring, this PR also adds ci.yml files to corresponding .sln files and updates the .sln files Visual Studio version used to VS 2022.

@konrad-jamrozik konrad-jamrozik added the Central-EngSys This issue is owned by the Engineering System team. label Dec 10, 2022
@konrad-jamrozik konrad-jamrozik self-assigned this Dec 10, 2022
@konrad-jamrozik konrad-jamrozik requested a review from a team as a code owner December 10, 2022 23:26
@konrad-jamrozik konrad-jamrozik changed the title Pin .NET to v6.0 for version-guard and CreateRuleFabricBot (+ Tests) projects + minor updates to .sln files Set .NET to v6.0 for version-guard and CreateRuleFabricBot (+ Tests) projects + minor updates to .sln files Dec 10, 2022
@konrad-jamrozik konrad-jamrozik changed the title Set .NET to v6.0 for version-guard and CreateRuleFabricBot (+ Tests) projects + minor updates to .sln files Set .NET used by our tools to v6.0 + minor updates to .sln files Dec 12, 2022
@konrad-jamrozik konrad-jamrozik changed the title Set .NET used by our tools to v6.0 + minor updates to .sln files Set .NET used by our tools to 'net6.0' + minor updates to .sln files Dec 12, 2022
@konrad-jamrozik konrad-jamrozik changed the title Set .NET used by our tools to 'net6.0' + minor updates to .sln files Set .NET used by our tools to net6.0 + minor updates to .sln files Dec 12, 2022
Copy link
Member

@mikeharder mikeharder left a comment

Choose a reason for hiding this comment

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

Approving changes under tools/http-fault-injector

Copy link
Member

@benbp benbp left a comment

Choose a reason for hiding this comment

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

+1 thank you for doing this! For check-enforcer and possibly webhook router, we can start to delete those projects entirely, I think. @hallipr just to double check, nothing of yours relies on the webhook-router eventhubs, right?

@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented Dec 13, 2022

@benbp

For check-enforcer and possibly webhook router, we can start to delete those projects entirely, I think.

If so, I will make a separate PR to delete these projects.

@weshaggard I am merging, assuming Ben's approval is enough. Let me know in case that's incorrect assumption xD

@konrad-jamrozik konrad-jamrozik merged commit 28c8db9 into main Dec 13, 2022
@konrad-jamrozik konrad-jamrozik deleted the users/kojamroz/iss_4934_pr1 branch December 13, 2022 06:14
Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

LGTM

@konrad-jamrozik
Copy link
Contributor Author

@benbp @hallipr created PRs to delete the two obsolete projects:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants