-
Notifications
You must be signed in to change notification settings - Fork 15.6k
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
C# AddressBook example and/or documentation not adequate to get started with version 3.5.1 #4119
Comments
I can't reproduce this, I'm afraid. I've just downloaded the 3.5.1 zip file, opened the solution file in VS 15.5.1 and run the AddressBook sample and it came up with a prompt as expected. What happens if you try to run the app from an open command line window instead? Have you tried setting a break point at the very start of Main instead of on line 52? |
Thanks for your immediate reply, @jskeet. Your affirmation that it worked in your environment encouraged me to look closely at the tools. Turns out that post-.Net-Core-2.0 releases of VS2017 require opting in to support the earlier versions of that framework, and under a somewhat misleadingly narrow label. There is no backward compatibility support within the IDE by the .NET Core 2.0 bits. (Perhaps the VS team wishes to turn the page on their 1.x alpha-like tooling support for .Net Core as quickly as possible.) It might be worth highlighting this in the ReadMe.md for the C# solution, where you say,
For example, you could add the sentence:
Incidentally, my offer remains open to assist with the migration of the AddressBook project to .Net Core 2.x once the time comes. I would also be interested in evaluating/testing the relevance of the new memory buffer-oriented C# 7.2 bits to the core code in the C# version of the Google.Protobuf itself. Would it make sense to open that as a separate issue or is that ultra low priority at the moment? |
Re: Span: there's already a feature request for it: #3431 Your readme suggestion sounds good to me - if you could file a PR for that, it would be good. "Migrating" the AddressBook project to .NET Core 2.x would be a single character change: in AddressBook.csproj, just change netcoreapp1.0 to netcoreapp2.0. I'd expect that to work with no other changes - from VS2017. However, I believe our CI systems etc are still using the 1.0 SDK. Updating to use the 2.0 SDK would require a non-trivial amount of work in my experience, as we'd still need to also install the 1.0 runtime in order to run 1.0-based tests. |
Thanks for your support with that minor edit to the ReadMe @jskeet, which appears to be on its way. I'll close this issue as soon as a change is completed. On the one-character migration (!): that seems like the reasonable expectation. Hopefully the VS tools will support that path by the time it makes sense for protobuf to make the upgrade. Regarding C# 7.2, I've followed up with a brief note on the feature request discussion you linked to see when (and whether) there would be value in moving forward with that. |
ok to close this issue now? The linked PR was merged. |
Steps to repro:
Results:
A. Builds okay.
B. After F5 run, a brief (<100 ms) screen flash appears of a console window opening and closing.
C. Nothing on the Debug output pane.
D. No errors.
E. Breakpoint not hit.
Expected result:
Hitting the breakpoint if set or alternatively have the console window appear and stay up.
Documentation consulted:
I'm sure I'm missing something obvious. Is running the csharp example in VS2017 a supported scenario? I have not yet tried to create a replacement AddressBook2 project using alternative dependencies such as the full framework or NET Core 2.04.
Happy to pitch-in in getting the csharp example up and running or in improving the docs if someone can provide some initial guidance as to any known issues.
UPDATE - FURTHER OBSERVATIONS 1/2/2018:
I would be happy to prepare a pull-request based on number 2 (above) to get the AddressBook project working and simultaneously migrate it to NETCore 2.0, a far more viable framework in any case. I will be on holiday from mid-January, so please let me know sooner rather than later if this would be useful, (@jskeet).
The text was updated successfully, but these errors were encountered: