Rename all the service reference things #9559
Conversation
|
Pity GitHub doesn't recognize the .props and .targets files were renamed before things in them were renamed. If you're primarily interested in the delta from existing code, I suggest git checkout dougbu/code.generation.renames.7492
git show --find-renames=20 -- *.props
git show --find-renames=20 -- *.targets |
Eilon
added
the
area-mvc
|
@natemcmaster I know you found one typo. Anything else ? |
dougbu
force-pushed
the
dougbu/code.generation.renames.7492
branch
from
7e28551 to
ab36245
Compare
...xtensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.targets
Outdated
Show resolved
Hide resolved
...xtensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.targets
Outdated
Show resolved
Hide resolved
...xtensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.targets
Outdated
Show resolved
Hide resolved
...xtensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.targets
Outdated
Show resolved
Hide resolved
...xtensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.targets
Show resolved
Hide resolved
...xtensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.targets
Outdated
Show resolved
Hide resolved
...xtensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.targets
Outdated
Show resolved
Hide resolved
- #7492 - remove document generation from client project; will be included in Web API project infrastructure (coming soon) Also - provide `%(FirstForGenerator)` metadata, #4916 - add `$(OpenApiGenerateAtDesignTime)` property (default `true` for now), #4944 - generate code in `obj` directory by default, #4945
- adjust eng/ProjectReferences.props to new project name - simplify `_CreateOpenApiReferenceItemsForOpenApiProjectReferences` target e.g. remove batching - simplify item de-duplication in `_CreateCompileItemsForServiceFileReferences` target i.e. use `Remove` attribute - also handle `.tsx` files in this target - remove malformed `Exists(...)` check in `_GenerateOpenApiReferenceCode` target nits: - remove a useless `StringBuilder.Append(...)` call - remove `%(OpenApiProjectReference.SourceProject)` metadata, duplicated `%(OriginalItemSpec)` - be more consistent about using element syntax for item metadata
dougbu
force-pushed
the
dougbu/code.generation.renames.7492
branch
from
ab36245 to
2cff475
Compare
- set `%(FirstForGenerator)` based on `%(CodeGenerator)` metadata Side note, PR fixes include - add `%(FirstForGenerator)` metadata, #4916 - add support for disabling design-time builds, #4944 - change default output directory to `obj/`, #4945 1 of 2 (remainder will come with #8242) - provide a default `%(CodeGenerator)` value, ##7491 1 of 2 (remainder will come in next milestone)
|
|
||
| <Compile Include="@(_Files)" | ||
| Exclude="@(Compile)" | ||
| Condition="'$(DefaultLanguageSourceExtension)' != '.ts' AND '%(_Files.OutputPathExtension)' == '$(DefaultLanguageSourceExtension)'"> |
There was a problem hiding this comment.
Have we tested this with F# projects? If not, should we limit this to 'Condition="'$(Language)'=='C#'"`?
There was a problem hiding this comment.
This condition is correct because compiled languages other than TypeScript are not mixed into other projects. That is, this is about the exception to normal mono-language projects.
| Do not change %(ReferenceOutputAssembly) if project contains duplicate @(ProjectReference) and | ||
| @(OpenApiProjectReference) items. | ||
| --> | ||
| <ProjectReference Include="@(OpenApiProjectReference)" Exclude="@(ProjectReference)"> |
There was a problem hiding this comment.
Does this work if the project have different TFMs? Like does it work if you reference a netcoreapp project from a netstandard project? I know we struggled with this when we were authoring test projects in Razor.
There was a problem hiding this comment.
It doesn't work perfectly yet and will be refined. As-is, it brings a large reliability improvement over explicit <MSBuild /> tasks to restore and build the server project.
For later consideration, could you point me to the Razor solution to this issue?
| <NoWarn>NU1702</NoWarn> | ||
| <ReferenceOutputAssembly>false</ReferenceOutputAssembly> | ||
| </ProjectReference> | ||
| <ProjectReference Update="@(OpenApiProjectReference)"> |
There was a problem hiding this comment.
Any reason not to inline this with the previous include?
There was a problem hiding this comment.
Because when @(ProjectReference) and @(OpenApiProjectReference) overlap we need to get the documents. The previous element is about the non-overlapping case and this covers everything.
There was a problem hiding this comment.
MSBuild and the SDK don't handle duplicate <ProjectReference> items well. This can cause some strange behaviors such as dotnet/msbuild#2688. Consider making this overlap an error.
There was a problem hiding this comment.
The two blocks are separate specifically to avoid creating duplicate <ProjectReference> items. Why make overlap between @(ProjectReference) and @(OpenApiProjectReference) an error with that in place?
| @@ -21,9 +21,5 @@ | |||
| <file src="buildMultiTargeting\*" target="buildMultiTargeting" /> | |||
| <file src="bin\$configuration$\net461\Microsoft.Extensions.ApiDescription.Tasks.*" target="tasks\net461" /> | |||
| <file src="bin\$configuration$\netstandard2.0\Microsoft.Extensions.ApiDescription.Tasks.*" target="tasks\netstandard2.0" /> | |||
| <file src="..\..\dotnet-getdocument\src\bin\$configuration$\netcoreapp2.1\publish\*.*" target="tools" /> | |||
There was a problem hiding this comment.
They aren't necessary with the new client / server split. The tool will come back in Microsoft.Extensions.ApiDescription.Server
| </GetFileReferenceMetadata> | ||
|
|
||
| <ItemGroup> | ||
| <OpenApiReference Remove="@(OpenApiReference)" /> |
There was a problem hiding this comment.
Should this be a different itemgroup rather than re-using OpenApiReference? It would be easier to debug this and wouldn't modify a user specified item.
There was a problem hiding this comment.
No, this is intended to flow @(OpenApiProjectReference) items into @(OpenApiReference). Besides, modifying user-specified items e.g. @(Compile) is just normal.
| absolute path. | ||
| --> | ||
| <OpenApiDefaultOutputDirectory | ||
| Condition="'$(OpenApiDefaultOutputDirectory)' == ''">$(BaseIntermediateOutputPath)</OpenApiDefaultOutputDirectory> |
There was a problem hiding this comment.
FYI - BaseIntermediateOutputPath might not be fully set when this file is evaluated. Would recommend moving this to a .targets file instead. BaseIntermediateOutputPath isn't defined until Microsoft.NET.DefaultOutputPaths.targets is evaluated, which happens at the end of project evaluation. Props file evaluation happens near the beginning of project eval.
There was a problem hiding this comment.
Hmm, it's working at the moment. I'll move it if we see issues or if I otherwise must update this PR.
| <NoWarn>NU1702</NoWarn> | ||
| <ReferenceOutputAssembly>false</ReferenceOutputAssembly> | ||
| </ProjectReference> | ||
| <ProjectReference Update="@(OpenApiProjectReference)"> |
There was a problem hiding this comment.
MSBuild and the SDK don't handle duplicate <ProjectReference> items well. This can cause some strange behaviors such as dotnet/msbuild#2688. Consider making this overlap an error.
Also
%(FirstForGenerator)metadata, Remove need for users to special-case second and later clients #4916$(OpenApiGenerateAtDesignTime)property (defaulttruefor now), Generated files re-appearing without explicit build #4944objdirectory by default, Change default document directory to obj #4945