Document .NET build integration with Grpc.Tools by kkm000 · Pull Request #15754 · grpc/grpc

kkm000 · 2018-06-14T05:14:31Z

This is to document features in #13207, and refers to examples changed in #14684, so depends on them both.

Explicitly refers to v1.14. Needs change if delayed past the cut-off.

listepo-alterpost · 2018-06-15T09:39:39Z

@kkm000 good job 👍

kkm000 · 2018-06-16T08:09:11Z

@listepo-alterpost, thank you, feel free to review and check the doc for typos, grammar and clarity. All comments are indeed welcome!

kkm000 · 2018-06-16T08:10:04Z

~~Rendered view: https://github.com/kkm000/grpc/blob/a1517ec6d4974dc01209e90dab383bca0151e95b/src/csharp/INTEGRATION.md~~

kkm000 · 2018-10-14T15:58:57Z

Latest view: https://github.com/kkm000/grpc/blob/27771aa60/src/csharp/INTEGRATION.md)

jtattermusch · 2018-10-17T15:58:39Z

src/csharp/INTEGRATION.md

+
+```xml
+  <ItemGroup>
+    <Protobuf Include="**/*.proto />


nit: missing "

Uh, nothing is a "nit" in the user-facing documentation! I'm also scrambling someone to proofread the text today.

jtattermusch · 2018-10-18T09:30:50Z

src/csharp/INTEGRATION.md

+### I just want to compile .proto files into my library
+
+This is the approach taken by the examples in the `csharp/examples` directory.
+Protoc output files (for example, `Hello.cs` and `HelloGrps.cs` compiled from


typo: HelloGrps

jtattermusch · 2018-10-18T09:31:53Z

src/csharp/INTEGRATION.md

+
+This is the approach taken by the examples in the `csharp/examples` directory.
+Protoc output files (for example, `Hello.cs` and `HelloGrps.cs` compiled from
+`hello.proto`) are placed among *object* and other temporary files of your


it's helloworld.proto and Helloworld.cs and HelloworldGrpc.cs

jtattermusch · 2018-10-18T09:36:28Z

src/csharp/INTEGRATION.md

+
+* "Classic" .csproj with `packages.config` (Visual Studio, Mono): This is
+ handled automatically by NuGet. See the attribute added to the
+ [helloworld/packages.config] file by Visual Studio.


btw, the layout of examples/csharp has been changed in the meantime. "Helloworld" now has the new-style csproj for dotnet SDK and "HelloworldLegacyCsproj" has the legacy projects. I usually call them "Legacy" rather than "Classic" btw.

jtattermusch · 2018-10-18T09:36:40Z

src/csharp/INTEGRATION.md

+
+* "Classic" .csproj with `packages.config` (Visual Studio, Mono): This is
+ handled automatically by NuGet. See the attribute added to the
+ [helloworld/packages.config] file by Visual Studio.


missing link to packages.config

jtattermusch · 2018-10-18T09:37:45Z

src/csharp/INTEGRATION.md

+ [Greeter.csproj](/examples/csharp/helloworld-from-cli/Greeter/Greeter.csproj#L9)
+ example project in this repository. If adding a package reference in Visual
+ Studio, edit the project file and add this attribute. [This is a bug in Nuget
+ clinet](https://github.com/NuGet/Home/issues/4125).


typo "clinet"

jtattermusch · 2018-10-18T09:39:15Z

src/csharp/INTEGRATION.md

+ Studio, edit the project file and add this attribute. [This is a bug in Nuget
+ clinet](https://github.com/NuGet/Home/issues/4125).
+
+If building a NuGet package from your library with a .nuspec file, then the spec


nit: add a not that .nuspec will be only used by legacy users, dotnet SDK users don't need to care anymore?

jtattermusch · 2018-10-18T09:39:58Z

src/csharp/INTEGRATION.md

+
+If building a NuGet package from your library with a .nuspec file, then the spec
+file may (and probably should) reference the Grpc metapackage, but **do not add
+a reference to Grpc.Tools** to it. "SDK" projects handle this automatically when


nit replace "SDK" with "dotnet SDK"?

jtattermusch · 2018-10-18T09:55:26Z

src/csharp/INTEGRATION.md

+* __Access__  
+Sets generated class access on _both_ generated message and gRPC stub classes.
+
+-=- End of INTEGRATION.md -=-


remove the end marker?

jtattermusch · 2018-10-18T10:01:41Z

src/csharp/INTEGRATION.md

@@ -0,0 +1,357 @@
+Protocol Buffers/gRPC Integration Into .NET Build


nit: the name of file INTEGRATION.md seems a bit too general - it's not immediately obvious what's meant by that.
How about CODEGEN.md or CODEGENERATION.md, CODEGEN-INTEGRATION.md, MSBUILD-INTEGRATION.md ....?

Most regular developers have no idea what MSBuild is, and are hardly aware that they are in fact using it when hitting Shift+Ctrl+B for "Build" in Visual Studio or typing dotnet build; I'd rather avoid mentioning it in a file name. The common theme is "build" here; BUILD-INTEGRATION.md maybe then?

Or TOOLS-NUGET-PACKAGE.md?

CODEGEN etc. sounds also less appealing, looking through the user's eyes. As in "I want to compile proto files." The documentation refers to protoc as the Protocol Buffer compiler; codegen may be perceived as just a teensy option to it. I'm afraid CODEGEN would not ring the bell when seen from the user's perspective.

But overall, I think that a link from README will go a long way toward rendering the name choice not very important. Absolutely most people read README files on GitHub these days, not looking for one after unpacking a tarball--not .NET users, certainly. Just let me know what name you pefer.

From your other comment:

I usually call them "Legacy" rather than "Classic" btw.

I never heard them called legacy anywhere in MS' own and related development. The operational jargon is "Classic projects" vs "SDK projects" (or s/project/build/ when speaking of the process, not file format). FWIW, the "classic" is not obsolete; they in fact now support ProjectReference, just like SDK projects, with NuGet tasks integrated into MBuild, in lieu of packages.conf (which relied on a plugin shipped with VS to modify the .csproj content). Or maybe I was just spending too much time digging through MSBuld, netcore and NuGet repositories while working on our thing. :)

One point statistics is not convincing, but here's a single example I just happened to hit first: a Googler discusses an issue with an MSBuild architect in this jargon: dotnet/msbuild#3546. I'd go with Classic even in the example name, if I were to choose. When I first seen it renamed to HelloWorldLegacy, it fell on my ear as not exactly right. Yes, I'm biased, but it seems I'm biased in the right direction this time. :)

I could not find any named "official" names in the documentation (docs.microsoft.com), so we are drawing a blank here. .NET project, Visual Stdio project. Boring. Even my cats have names, although they also do not care...

* Fix typos. * Correct pointers to example files. * Refer to v1.17 or 1.17.0 throughout. * Change wording where requested, remove trailer. * Rename INTEGRATION.md -> BUILD-INTEGRATION.md. * Change a pointer in README.md to point to the new name.

kkm000 · 2018-11-02T07:59:22Z

@jtattermusch, Done. I renamed the file BUILD-INTEGRATION.md. I believe it should be unambigous enough.

Render: https://github.com/kkm000/grpc/blob/f438d7c727/src/csharp/BUILD-INTEGRATION.md

The highlighted line numbers in the links to example files are off, as they refer to files in the examples PR #14684.

jtattermusch

LGTM. I'll merge soon and we can continue improving in followup PRs if needed. Thanks for such detailed docs!

jtattermusch · 2018-11-05T11:10:55Z

Docs only change, merging early.

grpc-testing · 2018-11-05T11:43:39Z

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 2,016,505      Total (=)      2,016,505

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
11,123,085      Total (>)     11,123,080

 No significant differences in binary sizes

kkm000 mentioned this pull request Jul 3, 2018

Remove protoc conditional properties protocolbuffers/protobuf#4817

Merged

jtattermusch self-assigned this Oct 2, 2018

jtattermusch added the lang/C# label Oct 2, 2018

Document .NET build integration with Grpc.Tools

27771aa

kkm000 force-pushed the package-grpc-tools-doc branch from a1517ec to 27771aa Compare October 14, 2018 15:45

kkm000 mentioned this pull request Oct 14, 2018

Integrate Grpc.Tools into msbuild system (C# only) #13207

Merged

jtattermusch reviewed Oct 17, 2018

View reviewed changes

Add missing quote

3100e94

jtattermusch reviewed Oct 18, 2018

View reviewed changes

Address review comments

4912fb2

* Fix typos. * Correct pointers to example files. * Refer to v1.17 or 1.17.0 throughout. * Change wording where requested, remove trailer. * Rename INTEGRATION.md -> BUILD-INTEGRATION.md. * Change a pointer in README.md to point to the new name.

kkm000 force-pushed the package-grpc-tools-doc branch from ed4adc6 to 4912fb2 Compare November 2, 2018 07:48

Merge branch master into package-grpc-tools-doc

f438d7c

jtattermusch added the kokoro:run label Nov 5, 2018

kokoro-team removed the kokoro:run label Nov 5, 2018

jtattermusch added the release notes: yes Indicates if PR needs to be in release notes label Nov 5, 2018

jtattermusch approved these changes Nov 5, 2018

View reviewed changes

jtattermusch merged commit 7109bd1 into grpc:master Nov 5, 2018

ghost mentioned this pull request Dec 6, 2018

grpcio versions available: 1.17.0 Harmon758/Harmonbot#494

Closed

lock bot locked as resolved and limited conversation to collaborators Feb 3, 2019

		@@ -0,0 +1,357 @@
		Protocol Buffers/gRPC Integration Into .NET Build

Conversation

kkm000 commented Jun 14, 2018

Uh oh!

listepo-alterpost commented Jun 15, 2018

Uh oh!

kkm000 commented Jun 16, 2018

Uh oh!

kkm000 commented Jun 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kkm000 commented Oct 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkm000 commented Nov 2, 2018

Uh oh!

jtattermusch left a comment

Choose a reason for hiding this comment

Uh oh!

jtattermusch commented Nov 5, 2018

Uh oh!

grpc-testing commented Nov 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kkm000 commented Jun 16, 2018 •

edited

Loading