Skip to content

Add interface to SshCommand #1508

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

eugencowie
Copy link
Contributor

Changes:

  • Add ISshCommand interface containing all public properties and methods of SshCommand
  • Update SshClient methods to return ISshCommand instead of SshCommand

Closes #1507

@Rob-Hague
Copy link
Collaborator

I am a bit uneasy about this one because of the breaking changes it introduces

@k0ka
Copy link

k0ka commented Jul 31, 2025

what exact breaking change are you referring to? As I understand the only problem might occur if the user had a code like this

SshCommand command = client.CreateCommand("ls");

But this code would be quite rare as the modern preferred coding style is

var command = client.CreateCommand("ls");

Every release there are small breaking changes. This change won't be very big and would be caught during compile time.

We might introduce another interface ISshClient2 which returns ISshCommand instead of SshCommand. But it won't make much sense as we anyway have to replace the main interface ISsshClient with it.

@Rob-Hague
Copy link
Collaborator

Yes that is the breaking change I am referring to.

But this code would be quite rare as the modern preferred coding style is

It is not my preferred coding style :-)

The breaking changes in each release are considered based on how many people it is going to benefit vs inconvenience. You can see my thoughts on mocking the library here #1667 (comment), but in short, I don't consider it an important feature for which is worth inconveniencing an unknown but possibly large number of people.

If we want to compare the proposed change here against prior changes:

  1. Add interface to SftpFile #120 #812. IMO this was a mistake. Even so, it is less inconvenient than this change due to implicit conversion in foreach loops (so foreach (SftpFile file in client.ListDirectory(...)) still compiles)
  2. Added ExistsAsync and GetAsync to ISftpClient #1628. Also a breaking change, but since these interfaces seem to only exist for people to mock, then it is more like "you get what you asked for"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add interface for SshCommand
3 participants