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

Enable roslyn analyzers #1044

Merged
3 commits merged into from
Apr 21, 2020
Merged

Enable roslyn analyzers #1044

3 commits merged into from
Apr 21, 2020

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Apr 20, 2020

Add roslyn analyzers and corresponding rules to ensure compliance with compliance and fix basic errors spotted by first round of analyzers.

@hoyosjs hoyosjs requested a review from josalem April 20, 2020 09:41
@hoyosjs hoyosjs added the auto-merge Automatically merge PR once CI passes. label Apr 20, 2020
@ghost
Copy link

ghost commented Apr 20, 2020

Hello @hoyosjs!

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.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 1 minute. No worries though, I will be back when the time is right! 😉

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.

@hoyosjs hoyosjs removed the auto-merge Automatically merge PR once CI passes. label Apr 20, 2020
@hoyosjs
Copy link
Member Author

hoyosjs commented Apr 20, 2020

@msftbot merge after @josalem reviews

@ghost ghost added the auto-merge Automatically merge PR once CI passes. label Apr 20, 2020
@ghost
Copy link

ghost commented Apr 20, 2020

Hello @hoyosjs!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I'll only merge this pull request if it's approved by @josalem

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@josalem
Copy link
Contributor

josalem commented Apr 20, 2020

Is the expectation that we will merge this on red and follow up to fix the warnings that are causing the failed jobs in CI?

@hoyosjs
Copy link
Member Author

hoyosjs commented Apr 20, 2020

Is the expectation that we will merge this on red and follow up to fix the warnings that are causing the failed jobs in CI?

The merge doesn't happen on red CI. I didn't commit one of the files to add an exception for a false positive.

@ghost ghost merged commit a4afa82 into dotnet:master Apr 21, 2020
@hoyosjs hoyosjs deleted the juhoyosa/roslyn-analyzers branch April 22, 2020 23:42
mdh1418 added a commit to mdh1418/runtime that referenced this pull request Feb 19, 2022
mdh1418 added a commit to mdh1418/runtime that referenced this pull request Feb 22, 2022
mdh1418 added a commit to dotnet/runtime that referenced this pull request Mar 2, 2022
…ests on Android (#64358)

* [tests][eventpipe] Port over Dotnet/Diagnostics files for mobile eventpipe tests

Copied over files from src/Microsoft.Diagnostics.NETCore.Client based off of 99fab307

* [tests][eventpipe] Add TCP/IP logic for mobile eventpipe tests

* [tests] Remove Microsoft.Diagnostics.NETCore.Client package reference

* [tests][eventpipe] Downstream Diagnostics IpcTraceTest DiagnosticsClient bootstrap

dotnet/diagnostics#720

* [tests][eventpipe] Downstream Diagnostics roslyn analyzer IpcTraceTest change

dotnet/diagnostics#1044

* [tests][eventpipe] Enable TCPIP DiagnosticsClient in IpcTraceTest for Android

* [tests][eventpipe] Aesthetic IpcTraceTest modifications

* [tests][eventpipe] Disable subprocesses tests on Android

* [tests][eventpipe] Update processinfo

* [tests][eventpipe] Update processinfo2

* [tests][eventpipe] Update eventsourceerror

* [tests][eventpipe] Update bigevent

* [tests][eventpipe] Update buffersize

* [tests][eventpipe] Update rundownvalidation

* [tests][eventpipe] Update providervalidation

* [tests][eventpipe] Update gcdump

* [tests][JIT] Update debuginfo/tester

* [tests] Segment Microsoft.Diagnostics.NETCore.Client relevant tests for Linux arm coreclr

* Account for nonspecified RuntimeFlavor

* [tests] Moveup Default coreclr RuntimeFlavor property explicit declaration

* [tests] Duplicate Microsoft.Diagnostics.NETCore.Client dependent tests for Linux arm

* Fix debuginfo/tester test skip

* Temporarily enable bigevent on Linux arm and remove duplicate exclude

* Fix unaligned UTF16 string read in collect tracing EventPipe command.

Collect tracing 2 EventPipe command triggers an unaligned UTF16 string
read that could cause a SIGBUS on platforms not supporting unalinged
reads of UTF16 strings (so far only seen on 32-bit ARM Linux CI machine).

On CoreCLR this could even cause a unalinged int read due to
optimizations used in UTF8Encoding::GetByteCount.

* Revert "[tests] Duplicate Microsoft.Diagnostics.NETCore.Client dependent tests for Linux arm"

This reverts commit cb2cacd.

* Revert "[tests] Segment Microsoft.Diagnostics.NETCore.Client relevant tests for Linux arm coreclr"

This reverts commit dc29676.

* Revert "Fix debuginfo/tester test skip"

This reverts commit 1e90d7e.

Co-authored-by: Mitchell Hwang <[email protected]>
Co-authored-by: lateralusX <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2024
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-merge Automatically merge PR once CI passes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants