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

SLVS-1399 Implement IServerConnectionRepository #5663

Conversation

gabriela-trutan-sonarsource
Copy link
Contributor

@gabriela-trutan-sonarsource gabriela-trutan-sonarsource commented Sep 2, 2024

SLVS-1399

This PR targets this PR, because it depends on it for the implementation.

The server connections could be tested with the UI: if the file exists, the connections are shown in the manage binding dialog and manage connections dialog. Adding a connection with the UI is not implemented yet.

Example of a valid json:
{
"serverConnections": [
{
"id": "https://sonarcloud.io/organizations/myOrg",
"setting": {
"IsSmartNotificationsEnabled": false
},
"organizationKey": "mya",
"serverUri": null
}
]
}

@terraform-techuser terraform-techuser changed the title SLVS-1399 Implement IServerConnectionRepository SLVS-1399 Implement IServerConnectionRepository Sep 2, 2024
{
lock (LockObject)
{
try

Choose a reason for hiding this comment

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

I don't think try catch is needed here. Read... already does logging, mapper should not throw normally and TryWrite does not throw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's exactly the mapper that can throw, more exactly: GetServerConnectionsListJsonModel and, more specifically, the GetServerConnectionJsonModel

Base automatically changed from gt/extend-file-handler to feature/new-connected-mode September 3, 2024 09:27
@gabriela-trutan-sonarsource gabriela-trutan-sonarsource deleted the branch feature/new-connected-mode September 3, 2024 14:50
@@ -35,7 +35,7 @@ internal class ServerConnectionsRepositoryAdapter(IServerConnectionsRepository s
{
public List<Connection> GetAllConnections()
{
var connections = serverConnectionsRepository.GetAll();
serverConnectionsRepository.TryGetAll(out var connections);

Choose a reason for hiding this comment

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

the return value should not be ignored

Copy link

sonarqubecloud bot commented Sep 9, 2024

@gabriela-trutan-sonarsource gabriela-trutan-sonarsource merged commit c36a36d into feature/new-connected-mode Sep 12, 2024
3 checks passed
@gabriela-trutan-sonarsource gabriela-trutan-sonarsource deleted the gt/implement-server-connections-repo branch September 12, 2024 07:26
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.

2 participants