Skip to content

Update the version of GenAPI used and pass through correct arguments#364

Merged
MichaelSimons merged 3 commits intodotnet:mainfrom
chsienki:update_gen_api_version
Mar 28, 2022
Merged

Update the version of GenAPI used and pass through correct arguments#364
MichaelSimons merged 3 commits intodotnet:mainfrom
chsienki:update_gen_api_version

Conversation

@chsienki
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

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

The new GenAPI version utilizes c# 8.0 features which is not compatible with the generated projects. Can you please update the project template accordingly?

@chsienki
Copy link
Copy Markdown
Member Author

@MichaelSimons Good catch, I pulled this out from another branch and missed that.

The newer GenAPI tool understands nullable and generates with the annotations. I chose not to enable nullability in the generated projects and just suppress the warnings about using it in a non-null context (CS8632) as there seemed to be a lot of non trivial warnings added when enabling it (even after suppressing CS8597 for throw null).

Let me know if you'd rather switch that around.

<PropertyGroup>
<TargetFrameworks>#TargetFrameworks#</TargetFrameworks>
<LangVersion>latest</LangVersion>
<NoWarn>8632</NoWarn>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should include $(NoWarn). Please moved to https://github.com/dotnet/source-build-reference-packages/blob/main/src/referencePackages/Directory.Build.props#L47 so that all warning suppressions are centrally located. A comment should be added like the other NoWarns indicating the reason.

Without this the generated code has numerous warnings.

Suggested change
<NoWarn>8632</NoWarn>
<NoWarn>$(NoWarn);8632</NoWarn>


<PropertyGroup>
<TargetFrameworks>#TargetFrameworks#</TargetFrameworks>
<LangVersion>latest</LangVersion>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rethinking about the patterns used in this repo, I think this may be better defined in the common props - https://github.com/dotnet/source-build-reference-packages/blob/main/src/referencePackages/Directory.Build.props. This gives us more flexibility without having to edit/regen all projects. The projects should just contain specializations.

Copy link
Copy Markdown
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

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

Thanks for the tooling improvements.

@MichaelSimons MichaelSimons merged commit 9ff1056 into dotnet:main Mar 28, 2022
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.

2 participants