Feature/schemacompare scmp save #824
Conversation
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaComapreSaveScmpRequest.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareSaveScmpOperation.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareService.cs
Outdated
Show resolved
Hide resolved
.../Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareServiceTests.cs
Outdated
Show resolved
Hide resolved
test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareTestUtils.cs
Outdated
Show resolved
Hide resolved
.../Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareServiceTests.cs
Outdated
Show resolved
Hide resolved
.../Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareServiceTests.cs
Show resolved
Hide resolved
.../Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareServiceTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs
Outdated
Show resolved
Hide resolved
| return null; | ||
| } | ||
|
|
||
| connInfo.ConnectionDetails.DatabaseName = databaseName; |
There was a problem hiding this comment.
we're mutating the connInfo. Can't the caller do this? Or, make a copy?
There was a problem hiding this comment.
not new code but as I understand copy is done because connInfo created from ownerURI doesnt have db information... and i think it was not done in caller here just to avoid duplicating this line...
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs
Outdated
Show resolved
Hide resolved
|
|
||
| internal static string FormatScript(string script) | ||
| { | ||
| script = RemoveExcessWhitespace(script); |
There was a problem hiding this comment.
Need to be careful since some whitespace may be part of a string literal.
There was a problem hiding this comment.
Not new code, just moved to utils since it was used by multiple classes. from what I understand it only removes two or more consecutive ones... @kisantia can you please correct my explanation :)
There was a problem hiding this comment.
Yes it removes multiple consecutive spaces so that the diff in ADS doesn't display it as a difference. Having different whitespace was a problem because spaces are used to align columns, so a table with a new column with a longer name would the whole table as being different instead of just the new column.
| try | ||
| { | ||
| operation = new SchemaCompareSaveScmpOperation(parameters, sourceConnInfo, targetConnInfo); | ||
| operation.Execute(parameters.TaskExecutionMode); |
There was a problem hiding this comment.
Why make this method async if we're just going to block the thread? Should this be async?
There was a problem hiding this comment.
Execute is sync... CurrentSchemaCompareTask = Task.Run(async () => {} in line 243 starts a thread which is async and we dont wait on that thread (other than in test)
Am I understanding the question correctly?
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareService.cs
Show resolved
Hide resolved
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaComapreSaveScmpRequest.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareSaveScmpOperation.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareSaveScmpOperation.cs
Outdated
Show resolved
Hide resolved
…name hence preserving any "."/"[" in name and avoiding any string operations
Adding changes to add save scmp operation in tools service side along with some generic minimal refactoring. Will add more end to end tests once we have scmp load changes as well (Kim will send a PR for scmp load).