Skip to content

Document warning waves. #29283

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
May 11, 2022
Merged

Document warning waves. #29283

merged 12 commits into from
May 11, 2022

Conversation

@BillWagner
Copy link
Member

@BillWagner BillWagner commented May 4, 2022 •
edited
Loading

Document the warning wave warnings from C# 9 through C# 11 (as of preview 17.3).

While editing the page on compiler warnings and errors, fix all remaining open issues.

Fixes #20355
Fixes #24089
Fixes #26199
Fixes #27661
Fixes #27872
Fixes #27927
Fixes #28806
Fixes #29265

@BillWagner BillWagner requested a review from a team as a code owner May 4, 2022 21:03
@BillWagner BillWagner marked this pull request as draft May 4, 2022 21:06
BillWagner added 5 commits May 10, 2022 10:35
@BillWagner
first draft done.
af1befb 
First drat is complete.
@BillWagner
Combine all definite assignment warnings
dcca45e 
I think this reads better. It's much less redundant, and these different warnings are related.
@BillWagner
proofread and link from what's new
c66953d
@BillWagner
Update for warning level / analysis level
c6f5a31 
While making those edits, fix other open issues on this page.
@BillWagner
Improve SEO
3d90574 
Work to move portions of the messages and error code into H2s.
@BillWagner BillWagner marked this pull request as ready for review May 10, 2022 21:52
@BillWagner BillWagner requested review from gewarren and tdykstra May 10, 2022 21:53
BillWagner
BillWagner commented May 10, 2022
View reviewed changes
docs/csharp/language-reference/compiler-messages/warning-waves.md

:::code language="csharp" source="./snippets/WarningWaves/WaveFive.cs" id="QueryPrecedenceNoWarn":::

## Members must be fully assigned, use of unassigned variable (CS8880, CS8881, CS8882, CS8883, CS8884, CS8885, CS8886, CS8887)
Copy link
Member Author

@BillWagner BillWagner May 10, 2022

Choose a reason for hiding this comment

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

@tdykstra

This section was very redundant if I had a separate header for each warning. In this commit, I tried to put some key words shared among the messages and added all the IDs. I'm ambivalent about that format. What do you think? Do you have any better suggestion here?

Copy link
Contributor

@tdykstra tdykstra May 11, 2022

Choose a reason for hiding this comment

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

This makes sense to me, though I'm ambivalent about it too, as it makes for a very long heading. The only revision I might consider is to put the warning numbers first to match the sequence in the other headings. You could make it a range --

CS8880-8887

-- but I don't know if that would compromise SEO for 81-87

Copy link
Member Author

@BillWagner BillWagner May 12, 2022

Choose a reason for hiding this comment

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

Good point @tdykstra

For now, I'll leave it as merged. I'll check stats and make any updates next month. I'll see how many searches we get on the warning code, vs. direct traffic from the F1 service.

tdykstra
tdykstra approved these changes May 11, 2022
View reviewed changes
Copy link
Contributor

@tdykstra tdykstra left a comment

Choose a reason for hiding this comment

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

Looks great, I noted just a few formatting suggestions.

@BillWagner BillWagner merged commit 71f085c into dotnet:main May 11, 2022
@BillWagner BillWagner deleted the warning-wave branch May 11, 2022 22:38
KalleOlaviNiemitalo
KalleOlaviNiemitalo reviewed May 12, 2022
View reviewed changes
BillWagner added a commit to BillWagner/docs that referenced this pull request May 12, 2022
@BillWagner
address comments made after merging
c3dbbbe 
I received community comments on dotnet#29283 after merging.

This fixes those issues.
@BillWagner BillWagner mentioned this pull request May 12, 2022
Merged
BillWagner added a commit that referenced this pull request May 12, 2022
@BillWagner
address comments made after merging (#29435)
ab1417a 
I received community comments on #29283 after merging.

This fixes those issues.
@BillWagner BillWagner added the okr-freshness OKR: Freshness of content label May 12, 2022
@BillWagner BillWagner added okr-curation OKR: Curate content to remove outdated recommendations and removed okr-experience labels May 23, 2022
baronfel pushed a commit to baronfel/dotnet-docs that referenced this pull request Jun 21, 2022
@BillWagner @baronfel
address comments made after merging (dotnet#29435)
f6f774a 
I received community comments on dotnet#29283 after merging.

This fixes those issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdykstra tdykstra tdykstra approved these changes

@gewarren gewarren Awaiting requested review from gewarren

+1 more reviewer

@KalleOlaviNiemitalo KalleOlaviNiemitalo KalleOlaviNiemitalo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

dotnet-csharp/svc errors-warnings/subsvc lang-reference/subsvc okr-curation OKR: Curate content to remove outdated recommendations okr-freshness OKR: Freshness of content

Projects

None yet

Milestone

No milestone

4 participants

@BillWagner @tdykstra @KalleOlaviNiemitalo @dotnet-bot