Skip to content

Update SDK #41697

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

Merged
merged 12 commits into from
May 17, 2022
Merged

Update SDK #41697

merged 12 commits into from
May 17, 2022

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented May 16, 2022

No description provided.

@wtgodbe wtgodbe requested review from a team and dougbu as code owners May 16, 2022 17:04
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 16, 2022
@wtgodbe wtgodbe enabled auto-merge (squash) May 16, 2022 17:10
@wtgodbe
Copy link
Member Author

wtgodbe commented May 16, 2022

Starting to see CS8981:

D:\a_work\1\s\artifacts\obj\Microsoft.AspNetCore.Grpc.JsonTranscoding\Release\net7.0\Internal\Protos\Errors.cs(8,7): error CS8981: The type name 'pb' only contains lower-cased ascii characters. Such names may become reserved for the language. [D:\a_work\1\s\src\Grpc\JsonTranscoding\src\Microsoft.AspNetCore.Grpc.JsonTranscoding\Microsoft.AspNetCore.Grpc.JsonTranscoding.csproj]
D:\a_work\1\s\artifacts\obj\Microsoft.AspNetCore.Grpc.JsonTranscoding\Release\net7.0\Internal\Protos\Errors.cs(9,7): error CS8981: The type name 'pbc' only contains lower-cased ascii characters. Such names may become reserved for the language. [D:\a_work\1\s\src\Grpc\JsonTranscoding\src\Microsoft.AspNetCore.Grpc.JsonTranscoding\Microsoft.AspNetCore.Grpc.JsonTranscoding.csproj]
D:\a_work\1\s\artifacts\obj\Microsoft.AspNetCore.Grpc.JsonTranscoding\Release\net7.0\Internal\Protos\Errors.cs(10,7): error CS8981: The type name 'pbr' only contains lower-cased ascii characters. Such names may become reserved for the language. [D:\a_work\1\s\src\Grpc\JsonTranscoding\src\Microsoft.AspNetCore.Grpc.JsonTranscoding\Microsoft.AspNetCore.Grpc.JsonTranscoding.csproj]
D:\a_work\1\s\artifacts\obj\Microsoft.AspNetCore.Grpc.JsonTranscoding\Release\net7.0\Internal\Protos\Errors.cs(11,7): error CS8981: The type name 'scg' only contains lower-cased ascii characters. Such names may become reserved for the language. [D:\a_work\1\s\src\Grpc\JsonTranscoding\src\Microsoft.AspNetCore.Grpc.JsonTranscoding\Microsoft.AspNetCore.Grpc.JsonTranscoding.csproj]

Looks like that's this sample:

using pb = global::Google.Protobuf;
using pbc = global::Google.Protobuf.Collections;
using pbr = global::Google.Protobuf.Reflection;
using scg = global::System.Collections.Generic;
. The sample claims to be auto-generated but I don't see anything that looks like the mentioned variables in WeatherReport.proto - @JamesNK how should we update those vars?

@dougbu
Copy link
Contributor

dougbu commented May 16, 2022

More generally, why are the warnings exposed despite the clear indications at the top of the generated files that they're generated code❔ If we need to take action, suggest extending the suppressed warnings to include CS8981 if possible.

@wtgodbe
Copy link
Member Author

wtgodbe commented May 16, 2022

Hmm, maybe it wasn't the sample, it could be something in protobuf always auto-generates usings with lowercase naming (hence the error being from artifacts\obj\Microsoft.AspNetCore.Grpc.JsonTranscoding\Release\net7.0\Internal\Protos\Errors.cs) . @JamesNK do you know if that's the case?

@JamesNK
Copy link
Member

JamesNK commented May 16, 2022

I raised it as something to fix in Grpc.Tools here: grpc/grpc#29672

For now, this warning needs to be suppressed in csproj.

@JamesNK
Copy link
Member

JamesNK commented May 17, 2022

image

Warnings are showing up in the gRPC template test. I think you need to temporarily quartine it. I have created a PR to fix this in Protobuf sourcegen, but I think it'll take a couple of months to flow through to a package we can reference to fix this warning.

@wtgodbe wtgodbe requested a review from Pilchie as a code owner May 17, 2022 17:09
@wtgodbe
Copy link
Member Author

wtgodbe commented May 17, 2022

@JamesNK I opened #41716 & assigned it to both of us

@wtgodbe wtgodbe merged commit 62ca84b into main May 17, 2022
@wtgodbe wtgodbe deleted the wtgodbe/SDK516 branch May 17, 2022 21:22
@ghost ghost added this to the 7.0-preview5 milestone May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants