NewProjectScaffolding-modification-for-mgmt #5167

welovej · 2024-11-20T06:10:28Z

Description

Add your description here!

Checklist

To ensure a quick review and merge, please ensure:

The PR has a understandable title and description explaining the why and what.
The PR is opened in draft if not ready for review yet.
- If opened in draft, please allocate sufficient time (24 hours) after moving out of draft for review
The branch is recent enough to not have merge conflicts upon creation.

Ready to Land?

Build is completely green
- Submissions with test failures require tracking issue and approval of a CODEOWNER
At least one +1 review by a CODEOWNER
All -1 reviews are confirmed resolved by the reviewer
- Override/Marking reviews stale must be discussed with CODEOWNERS first

src/AutoRest.CSharp/Common/AutoRest/Plugins/NewProjectScaffolding.cs

…ng-modification-for-mgmt

live1206 · 2024-11-26T06:53:58Z

samples/AzureSample.ResourceManager.Sample/src/AzureSample.ResourceManager.Sample.csproj

-    <PackageTags>AzureSample.ResourceManager.Sample</PackageTags>
-    <TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
-    <IncludeOperationsSharedSource>true</IncludeOperationsSharedSource>
+    <PackageTags>azure;management;arm;resource manager;sample</PackageTags>


Take a look at a real sample project: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/cosmosdb/Azure.ResourceManager.CosmosDB/samples/Azure.ResourceManager.CosmosDB.Samples.csproj, the PackageTags and PackageId are not there.

src/AzureSample.ResourceManager.Sample.csproj.

Though it has keyword Sample, but it's under src directory and treat as src project not sample project.

live1206 · 2024-11-26T06:57:03Z

...les/AzureSample.ResourceManager.Sample/tests/AzureSample.ResourceManager.Sample.Tests.csproj

@@ -1,9 +1,5 @@
 <Project Sdk="Microsoft.NET.Sdk">


I am wondering what is the intention of this empty test project generation?

As normal, there are should be another two file existing : testbase and testenvironment which namespace lies in testframework. if so, some reference problmes would occur and hard to make a right package reference of testframework. so I blocked the geneartion of these two file in scaffolding but did not block the test project generation.

src/AutoRest.CSharp/Assets/AssemblyInfo_publickey.txt

live1206 · 2024-11-26T07:04:52Z

test/CadlRanchProjects/Directory.Build.props

@@ -10,7 +10,8 @@
    <IsTestGenerationTestProject Condition="$(MSBuildProjectName.EndsWith('.Tests'))">true</IsTestGenerationTestProject>
    <RequiredTargetFrameworks Condition="'$(IsTestGenerationSrcProject)' == 'true'">netstandard2.0</RequiredTargetFrameworks>
    <RequiredTargetFrameworks Condition="'$(IsTestGenerationTestProject)' == 'true'">net7.0;net6.0;net462</RequiredTargetFrameworks>


should this be net8.0? And then, do we still need the new one for mgmt?

yes. It should be net8.0

live1206 · 2024-11-26T09:20:21Z

src/AutoRest.CSharp/Common/AutoRest/Plugins/NewProjectScaffolding.cs

            {
                slnContent += @"		{{ECC730C1-4AEA-420C-916A-66B19B79E4DC}}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{{ECC730C1-4AEA-420C-916A-66B19B79E4DC}}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{{ECC730C1-4AEA-420C-916A-66B19B79E4DC}}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{{ECC730C1-4AEA-420C-916A-66B19B79E4DC}}.Release|Any CPU.Build.0 = Release|Any CPU
+";
+            }
+            if (Configuration.AzureArm && Configuration.Namespace.StartsWith("Azure.ResourceManager"))


why is this limited to Azure.ResourceManager?

In autorest.csharp, there are no proper project or package reference. If we generate sample.csproj and added in solution file, then the pipeline would have many syntax error. I restrict the namespace to "Azure.ResourceManager", so it would only generate sample's csproj file and added to the solution file under real sdk management generation environment.

How about adding a configuration "skip-test-project-scaffolding" to skip the test projects generation in autorest.csharp? Similar to

autorest.csharp/src/AutoRest.CSharp/Common/Input/Configuration.cs

Line 32 in c27e20f

public const string SkipCSProj = "skip-csproj";

That will be clearer.

live1206 · 2024-11-26T09:24:49Z

src/AutoRest.CSharp/Common/AutoRest/Plugins/NewProjectScaffolding.cs

            {
                slnContent += @"Project(""{{9A19103F-16F7-4668-BE54-9A1E7A4F7556}}"") = ""Azure.Core.TestFramework"", ""..\..\core\Azure.Core.TestFramework\src\Azure.Core.TestFramework.csproj"", ""{{ECC730C1-4AEA-420C-916A-66B19B79E4DC}}""
 EndProject
+";
+            }
+            if (Configuration.AzureArm && Configuration.Namespace.StartsWith("Azure.ResourceManager"))


why is Azure.ResourceManager needed here?

same reason as the above one.

live1206 · 2024-11-26T09:36:13Z

src/AutoRest.CSharp/Common/AutoRest/Plugins/NewProjectScaffolding.cs

                await File.WriteAllBytesAsync(Path.Combine(_projectDirectory, "CHANGELOG.md"), Encoding.ASCII.GetBytes(GetChangeLog()));
            }
        }
-
+        private string GetAssetJson()


why is this Json?

Do you mean the name is not good? maybe GetAssetsContent would be better?

adpoted: GetAssetsJson

live1206 · 2024-11-26T09:37:49Z

src/AutoRest.CSharp/Common/AutoRest/Plugins/NewProjectScaffolding.cs

@@ -333,7 +188,6 @@ private string GetBrandedSrcCSProj()

            return builder.Write();
        }
-


it would be better to keep new line between methods

ArcturusZhang · 2024-11-29T05:40:18Z

src/AutoRest.CSharp/Common/AutoRest/Plugins/NewProjectScaffolding.cs

            if (_isAzureSdk)
            {
-                Directory.CreateDirectory(Path.Combine(_testDirectory, "SessionRecords"));
+                Directory.CreateDirectory(Path.Combine(_testDirectory, "Scenario"));
            }


I think we no longer need this after the test proxy update for test framework.
Therefore we could remove this.

modified scaffolding

2a0d6d2

welovej requested review from m-nash, lirenhe, ArthurMa1978, hallipr and JoshLove-msft as code owners November 20, 2024 06:10

welovej added 3 commits November 20, 2024 14:17

Update NewProjectScaffolding.cs

49ab4b0

Update NewProjectScaffolding.cs

3cf55b6

Update NewProjectScaffolding.cs

2698c69

live1206 reviewed Nov 20, 2024

View reviewed changes

welovej added 20 commits November 20, 2024 16:15

WriteProperties refactor

235c44e

refactor txt reading

88a4da8

Delete assets.json

45fb5d8

asset update

89673d9

not needed

d213a8c

update

ff68e47

Update NewProjectScaffolding.cs

b419980

Update NewProjectScaffolding.cs

d16d4a1

Update NewProjectScaffolding.cs

0ba37bc

Update NewProjectScaffolding.cs

c324701

Update NewProjectScaffolding.cs

31acd2d

Update Directory.Build.props

ff189c3

Update Directory.Build.props

65686de

Update AssemblyInfo_InfoContent.txt

32fcacf

newline fixed

b9a017d

Update MgmtTypeSpec.sln

68d5c5c

Update Directory.Build.props

297205a

Update NewProjectScaffolding.cs

531eb2c

run generate.ps1

7238b86

Merge remote-tracking branch 'upstream/main' into NewProjectScaffoldi…

4b48ffd

…ng-modification-for-mgmt

welovej added 3 commits November 26, 2024 09:04

Update CSProjWriter.cs

4af7cb1

Update Packages.Data.props

e9fca6e

regen

70eb20c

welovej mentioned this pull request Nov 26, 2024

Leverage new project scaffolding for management-plane new service onboarding Azure/azure-sdk-for-net#46898

Open

live1206 reviewed Nov 26, 2024

View reviewed changes

welovej added 13 commits November 28, 2024 09:22

for review comment

d26d36c

revised according to comment.

fd3c542

GetAssetsJson

9b3e00c

SkipMgmtScaffoldingInAutoRestcSharp config added

18d5363

fix SkipMgmtScaffoldinginAutoRestcSharp

4706631

regen 1

744a1dd

regen2

451545b

Update StaticStrings.cs

fbce198

delete asset.json

8acd345

gen

9ddd53c

Update NewProjectScaffolding.cs

48b11c1

Update NewProjectScaffolding.cs

183d388

Update Azure.NewProject.TypeSpec.sln

4998cc9

ArcturusZhang reviewed Nov 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NewProjectScaffolding-modification-for-mgmt #5167

NewProjectScaffolding-modification-for-mgmt #5167

welovej commented Nov 20, 2024

live1206 Nov 26, 2024

welovej Nov 28, 2024

live1206 Nov 26, 2024

welovej Nov 27, 2024

live1206 Nov 26, 2024

welovej Nov 27, 2024

live1206 Nov 26, 2024 •

edited

Loading

welovej Nov 27, 2024

live1206 Nov 28, 2024 •

edited

Loading

live1206 Nov 26, 2024

welovej Nov 27, 2024

live1206 Nov 26, 2024

welovej Nov 27, 2024

welovej Nov 28, 2024

live1206 Nov 26, 2024

welovej Nov 27, 2024

ArcturusZhang Nov 29, 2024

		@@ -333,7 +188,6 @@ private string GetBrandedSrcCSProj()

		return builder.Write();
		}

NewProjectScaffolding-modification-for-mgmt #5167

Are you sure you want to change the base?

NewProjectScaffolding-modification-for-mgmt #5167

Conversation

welovej commented Nov 20, 2024

Description

Checklist

Ready to Land?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

live1206 Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

live1206 Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

live1206 Nov 26, 2024 •

edited

Loading

live1206 Nov 28, 2024 •

edited

Loading