Skip to content

Docs for CA2252 #26149

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

Merged
merged 3 commits into from
Sep 24, 2021
Merged

Docs for CA2252 #26149

merged 3 commits into from
Sep 24, 2021

Conversation

pgovind
Copy link

@pgovind pgovind commented Sep 15, 2021

Documentation for CA2252 behavior. Followed instructions from https://docs.microsoft.com/en-us/contribute/dotnet/dotnet-contribute-code-analysis. Hopefully I got the grammar/formatting rules right!

cc @jeffhandley @terrajobst

@pgovind pgovind requested a review from a team as a code owner September 15, 2021 20:49
@dotnet-bot dotnet-bot added this to the September 2021 milestone Sep 15, 2021
<PropertyGroup>
<EnablePreviewFeatures>true</EnablePreviewFeatures>
</PropertyGroup>
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```

Copy link
Author

Choose a reason for hiding this comment

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

Did you actually mean to delete the ``` here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this must be a GitHub bug. The actual suggestion is to indent the ``` by two spaces.

Copy link
Member

Choose a reason for hiding this comment

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

@gewarren Using backticks inside backticks don't always work well. Whenever I do suggestions involving backticks I use:

~~~suggestion
suggestion containing ``` here
~~~

instead of

```suggestion
suggestion containing ``` here
```

The following is using ~~~suggestion instead of ``````````suggestion`

Suggested change
```
```

Just in case you come across this in future, it has bothered me for long before I knew about the ~~~

Copy link
Contributor

Choose a reason for hiding this comment

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

That's awesome, thanks @Youssef1313 !

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

I agree with all of @gewarren's suggestions. Assuming those get applied, I have no other feedback. 👍

@jeffhandley
Copy link
Member

@pgovind We should include a section about the side effects of <EnablePreviewFeatures>true</> and also cover <GenerateRequiresPreviewFeaturesAttribute> and its behavior. I could even imagine a table that shows the net result of each combination and what scenario the combination is useful for.

This might have helped with dotnet/aspnetcore#36783 (comment).

@gewarren
Copy link
Contributor

I do have a reference section for both of the MSBuild properties. Maybe you can expand those sections and then link to it?

https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#enablepreviewfeatures

@pgovind
Copy link
Author

pgovind commented Sep 24, 2021

The markdown leg failure here is unrelated. I made a change to msbuild-props.md in the latest commit. It;s a minor change, should be an easy review.

@gewarren
Copy link
Contributor

Thanks for the ping, I'll merge this now.

@gewarren gewarren merged commit 3004781 into dotnet:main Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants