-
Notifications
You must be signed in to change notification settings - Fork 87
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
Use LangVersion=8 where feasible in testing #1217
Conversation
trying a close and reopen. #1218 has already been merged, and that removes the attributes added in 9. Since there aren't any conflicts, that may be enough. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM @jskeet
Once the build is clean, let's
I'll rebase and repush. |
This should allow us to avoid accidentally using C# 9+ features, at least in most cases. The standalone-console and standalone-console-without-using templates are unchanged, as many examples using them expect to use top-level statements. (Once we've got as far as C# 10, we should be able to use LangVersion for all projects.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a suggestion to fix the build issue. Then, I think this is ready.
Co-authored-by: Bill Wagner <wiwagn@microsoft.com>
Merged the suggestion - let's see whether we get to green... |
Fixed a typo, so let's wait another time... |
It's green :) Merging. |
Note that now both MaybeNull1Attribute and MaybeNull2Attribute fail to build - I believe this is correct, and should be reflected in spec changes which you may be doing already. Happy to wait until you're done, then rebase this.