Skip to content
This repository was archived by the owner on Apr 9, 2021. It is now read-only.

.NET SDK integration post, NuGet v1.17#775

Merged
jtattermusch merged 5 commits intogrpc:masterfrom
kkm000:kkm/grpc-dotnet-build
Dec 18, 2018
Merged

.NET SDK integration post, NuGet v1.17#775
jtattermusch merged 5 commits intogrpc:masterfrom
kkm000:kkm/grpc-dotnet-build

Conversation

@kkm000
Copy link
Copy Markdown

@kkm000 kkm000 commented Nov 10, 2018

This is a post announcing integrated build of proto files in a C# .NET projects, upcoming in v1.17.

I hope I matched both the general philosophy and tone of the blog, but I'd be happy to make any corrections.

/cc @jtattermusch

Copy link
Copy Markdown

@carl-mastrangelo carl-mastrangelo left a comment

Choose a reason for hiding this comment

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

@kkm000 Looks cool, I did a brief pass over it. Please ping this if it isn't getting attention.

Comment thread _posts/2018-11-16-grpc-dotnet-build.md
Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
drwxrwxr-x 2 kkm kkm 4096 Nov 9 16:10 obj/
```

The `dotnet new` command has created the file `Class1.cs` that we won't need, so
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: it's easier to read if the verb tenses align throughout. Consider rewording this "The dotnew new command creates the file ...".

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I rephrased that sentence a bit, but I am not sure I grok your concern. This is as present a tense as it only could be.

Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
Copy link
Copy Markdown

@hsaliak hsaliak left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for writing it up! left a few minor comments

Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
Comment thread _posts/2018-11-16-grpc-dotnet-build.md
Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
Comment thread _posts/2018-11-16-grpc-dotnet-build.md
Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
<PackageReference Include="Google.Protobuf" Version="3.6.1" />
<PackageReference Include="Grpc" Version="1.17.0" />
<PackageReference Include="Grpc.Tools" Version="1.17.0" PrivateAssets="All" />
<ProtoBuf Include="helloworld.proto" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: put the tag under a separate ? That would make it more visible and it's also the style that Visual Studio would use if you added a file through the IDE.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Come think of it, I'm rather inclined against this.

My thinking goes, I do not know if user's editor supports XML, and XML is nasty to type without syntax highlighting and checking from the editor. So I think for the purposes of the throwaway walkthrough this is good enough. The goal as I understand it is to make the walkthrough as smooth as possible, and XML brockets are braindead. By me, the fewer, the better.

Some people prefer to keep it neat, others just lump it all together. And MSBuild does not care.

Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Nov 15, 2018

In summary, I added a 2nd paragraph from the top, above the more marker, to explain the intent better, another paragraph on dependency checking, and one sentence st the very end--it sounded to me as if it was lacking a nice closing.

Thanks for the comments everyone! Ready for the round 2!

@jtattermusch, this should not be published before the example branch is merged, I guess?

@jtattermusch
Copy link
Copy Markdown

@kkm000 yes this will only be published once 1.17.0 packages are out, so that the feature can be easily tried by everyone.

Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
Comment thread _posts/2018-11-16-grpc-dotnet-build.md
Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
Comment thread _posts/2018-11-16-grpc-dotnet-build.md Outdated
<ItemGroup>
<PackageReference Include="Google.Protobuf" Version="3.6.1" />
<PackageReference Include="Grpc" Version="1.17.0" />
<!-- Add attribute as shown below. -->
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: at the first sight the indent looks like it's done by mistake. Also let's make the comment more informative.

<!-- mark Grpc.Tools as build-only dependency by adding PrivateAssets="All" attribute -->

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, does this read better? Not really sure if the shouty-cased "IMPORTANT" is appropriate.

Copy link
Copy Markdown

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

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

I think the blogpost is looking good, thanks for writing it!

I'll merge (and publish the blogpost) once the v1.17.0 release is out, nugets are pushed and the examples PR is merged.

@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 4, 2018

Super, thank you!

@carl-mastrangelo
Copy link
Copy Markdown

This post will tentatively go out tomorrow. @kkm000, would you be able to rename the post to 12-18-18 so that it appears as the newest post on the https://grpc.io/blog/ list?

@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 18, 2018

@carl-mastrangelo Done.

@jtattermusch
Copy link
Copy Markdown

Let's merge?

@jtattermusch
Copy link
Copy Markdown

Things are looking good, so I'll go ahead and merge.

@jtattermusch jtattermusch merged commit ae762c0 into grpc:master Dec 18, 2018
@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 18, 2018

Looks weird, no heading or the pre-cut paragraphs. Is there some indexing that has to happen, or is something wrong with formatting?

@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 18, 2018

Just realized, this: https://github.com/grpc/grpc.github.io/blob/master/_posts/2018-18-18-grpc-dotnet-build.md shows the keywords as a block, but this https://github.com/grpc/grpc.github.io/blob/master/_posts/2018-12-11-grpc-stacks.md as table. The parser does not like something. Quotes around 'kkm'? No other ideas, really!

@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 18, 2018

@jtattermusch
Copy link
Copy Markdown

Good catch, do you wanna create a PR to replace : with -?

@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 18, 2018

Here is a couple of solutions, trying both. Quotes seem less ugly that the hex code. Just a sec.

@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 18, 2018

jekyll/jekyll#549

@kkm000
Copy link
Copy Markdown
Author

kkm000 commented Dec 18, 2018

@jtattermusch, #798. Looks like a table, similar to other .ms files.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants