Skip to content
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

[release/6.0-preview7] Update analyzer rulesets #56039

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jul 20, 2021

Backport of #55925 to release/6.0-preview7

/cc @pgovind @stephentoub

Customer Impact

The latest analyzer bits will cause dotnet/runtime's build to fail with warnings. With dotnet/sdk#19084, the latest analyzer bits will make it to the sdk's P7 branch. I'm not sure if an update to the sdk's branch will trigger a dotnet/runtime build, but I want to be safe here anyway and turn off the DetectPreviewFeatures analyzer for dotnet/runtime. We'll turn on preview features and iterate on the analyzer over the RC1 timeframe to fix the warnings for RC1

Testing

No product change here, not sure there's anything to test

Risk

Minimal. We're already doing this in upstream/main.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@danmoseley
Copy link
Member

danmoseley commented Jul 20, 2021

You're disabling many more analyzers than just the preview one. Is that right?

If this is necessary for the build to not be broken, we don't need Steve's approval. You can merge it when it's signed off.

@pgovind pgovind force-pushed the backport/pr-55925-to-release/6.0-preview7 branch from b464ab7 to 753399c Compare July 20, 2021 23:49
@pgovind pgovind requested a review from stephentoub July 20, 2021 23:51
@pgovind
Copy link

pgovind commented Jul 21, 2021

@stephentoub : I pared your change down to just disabling CA2252. Sign off when you get a chance please?

@ghost
Copy link

ghost commented Jul 21, 2021

Hello @pgovind!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost
Copy link

ghost commented Jul 21, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #55925 to release/6.0-preview7

/cc @pgovind @stephentoub

Customer Impact

The latest analyzer bits will cause dotnet/runtime's build to fail with warnings. With dotnet/sdk#19084, the latest analyzer bits will make it to the sdk's P7 branch. I'm not sure if an update to the sdk's branch will trigger a dotnet/runtime build, but I want to be safe here anyway and turn off the DetectPreviewFeatures analyzer for dotnet/runtime. We'll turn on preview features and iterate on the analyzer over the RC1 timeframe to fix the warnings for RC1

Testing

No product change here, not sure there's anything to test

Risk

Minimal. We're already doing this in upstream/main.

Author: github-actions[bot]
Assignees: -
Labels:

area-Infrastructure-libraries, auto-merge

Milestone: -

@pgovind
Copy link

pgovind commented Jul 21, 2021

@danmoseley : When you get a chance, can you merge this PR please? (It looks like I'm not authorized to merge it?) The test failures are websocket HTTP stuff unrelated to the change.

@danmoseley
Copy link
Member

I'm not either, Please ask on the infra channel and someone will do it promptly.

@safern
Copy link
Member

safern commented Jul 21, 2021

I can take care of it if needed, just let me know when the build is done.

@ghost
Copy link

ghost commented Jul 21, 2021

Apologies, while this PR appears ready to be merged, it looks like release/6.0-preview7 is a protected branch and I have not been granted permission to perform the merge.

1 similar comment
@ghost
Copy link

ghost commented Jul 21, 2021

Apologies, while this PR appears ready to be merged, it looks like release/6.0-preview7 is a protected branch and I have not been granted permission to perform the merge.

@safern safern merged commit 640d3ea into release/6.0-preview7 Jul 21, 2021
@safern safern deleted the backport/pr-55925-to-release/6.0-preview7 branch July 21, 2021 19:33
@pgovind
Copy link

pgovind commented Jul 21, 2021

Thanks @safern !

@ghost ghost locked as resolved and limited conversation to collaborators Aug 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants