SLVS-1378 Binding Dialog: Trigger binding #5695
Conversation
The IUnintrusiveBindingController interface is internal, which causes issues as it has to be accessed by the ConnectedMode assembly. It's not possible to change it to public, because of the access modifiers of its parameters. So the IBindingController is added to overcome these limitations.
vnaskos-sonar
changed the base branch from
master
to
feature/new-connected-mode
vnaskos-sonar
force-pushed
the
vn/bind
branch
from
ba858ac to
f3eebc1
Compare
| @@ -33,10 +34,10 @@ public partial class ManageBindingDialog : Window | |||
| { | |||
| private readonly IConnectedModeServices connectedModeServices; | |||
|
|
|||
| public ManageBindingDialog(IConnectedModeServices connectedModeServices, ISolutionInfoProvider solutionInfoProvider) | |||
| public ManageBindingDialog(IConnectedModeServices connectedModeServices, IBindingController bindingController, ISolutionInfoProvider solutionInfoProvider) | |||
There was a problem hiding this comment.
We already have the need to have more services than the ones provided by the IConnectedModeServices. Should we add another one that wraps the IBindingController and ISolutionInfoProvider?
It could have a name like "ConnectedModeBindingServices"
There was a problem hiding this comment.
I would leave that for a later stage on a separate PR when we finish adding new features, so we have a better view of what refactorings are needed.
| @@ -247,4 +243,31 @@ internal bool LoadConnections() | |||
| BoundProject = SelectedProject; | |||
| return new AdapterResponse(BoundProject != null); | |||
| } | |||
|
|
|||
| internal /* for testing */ async Task<AdapterResponse> BindAsync() | |||
There was a problem hiding this comment.
We don't need the comment /* for testing*/ as it is confusing. This is a code that is NOT used only for testing.
There was a problem hiding this comment.
The BindAsync should have been private, but the architecture/design does not allow it to be testable (in this case due to the ProgressReporter.ExecuteTaskWithProgressAsync.)
The meaning of internal /* for testing */ is that this method is internal only for testing reasons. We have this in many places in the codebase, which also acts as a reminder for possible future improvements.
I'll keep it until we have a documented convention that cancels it out or a better solution.
There was a problem hiding this comment.
I understand. I do find this solution unnecessary, though, so I will add it to the weekly discussion
src/ConnectedMode.UnitTests/UI/ManageBinding/ManageBindingViewModelTests.cs
Outdated
Show resolved
Hide resolved
vnaskos-sonar
force-pushed
the
vn/bind
branch
from
0f5e737 to
13186a0
Compare
|
| var connectionInformation = project.ServerConnection.Credentials.CreateConnectionInformation(project.ServerConnection.ServerUri); | ||
| await sonarQubeService.ConnectAsync(connectionInformation, cancellationToken); | ||
| await BindAsync(project, null, cancellationToken); | ||
| activeSolutionChangedHandler.HandleBindingChange(false); |
There was a problem hiding this comment.
I think we should add this to the other BindAsync overload, so that the BindWithMigrationAsync also has access to it
vnaskos-sonar
merged commit 515857a
into
feature/new-connected-mode
SLVS-1378