Skip to content

Fix analyzer errors in Renci.SshNet and Renci.SshNet.TestTools.OpenSSH - #1229

Merged
WojciechNagorski merged 9 commits into
developfrom
3rd-time-is-a-sharm
Nov 1, 2023
Merged

Fix analyzer errors in Renci.SshNet and Renci.SshNet.TestTools.OpenSSH#1229
WojciechNagorski merged 9 commits into
developfrom
3rd-time-is-a-sharm

Conversation

@drieseng

@driesengdrieseng commented Oct 29, 2023

Copy link
Copy Markdown
Member

Fix analyzer errors in Renci.SshNet and Renci.SshNet.TestTools.OpenSSH.
Suppress all errors in unit tests and integration tests.

@driesengdrieseng changed the title 3rd time is a sharmFix analyzer errors in Renci.SshNet and Renci.SshNet.TestTools.OpenSSHOct 29, 2023
Rob-Hague
Rob-Hague reviewed Oct 30, 2023
View reviewed changes

@Rob-HagueRob-Hague left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I still don't see any value in enabling analyzers just to add a bunch of suppressions in the source, but otherwise it is fine for me.

Comment threadsrc/Renci.SshNet/Channels/Channel.cs Outdated
Comment threadsrc/Renci.SshNet/Common/SshData.cs Outdated
Comment threadsrc/Renci.SshNet/Common/SshData.cs Outdated
Comment threadsrc/Renci.SshNet/Renci.SshNet.csproj Outdated
Comment threadsrc/Renci.SshNet/SftpClient.cs Outdated
Rob-Hague
Rob-Hague reviewed Oct 31, 2023
View reviewed changes
Comment threadsrc/Renci.SshNet/Channels/Channel.cs Outdated
@drieseng

drieseng commented Nov 1, 2023

Copy link
Copy Markdown
MemberAuthor

@WojciechNagorski, can I also get your stamp of approval?

@WojciechNagorski
WojciechNagorski merged commit 508fc87 into developNov 1, 2023
@WojciechNagorski
WojciechNagorski deleted the 3rd-time-is-a-sharm branch November 1, 2023 10:33
@Rob-Hague

Rob-Hague commented Nov 2, 2023

Copy link
Copy Markdown
Collaborator

Build times and general responsiveness in VS have taken a massive hit. I beg that we reconsider 🥺

@drieseng

drieseng commented Nov 3, 2023

Copy link
Copy Markdown
MemberAuthor

@Rob-Hague, I'm perfectly fine with disabling (some of) the analyzers by default. I'll submit a new PR to get the build in an acceptable state.

@Rob-HagueRob-Hague mentioned this pull request Nov 13, 2023
Closed
@WojciechNagorskiWojciechNagorski mentioned this pull request Dec 29, 2023
Closed
This was referenced Aug 12, 2026
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Aug 12, 2026
Open
Merged
Open
Open
Open
Open
Open
Open
Open
Closed
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Merged
Open
Open
Merged
Open
Merged
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Rob-HagueRob-HagueRob-Hague approved these changes
@WojciechNagorskiWojciechNagorskiWojciechNagorski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@drieseng@Rob-Hague@WojciechNagorski