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.

Sign up for GitHub

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

Jump to bottom

OptionsMapTable preparing from DacFx description,value,displayname at… #1550

Merged

ssreerama merged 16 commits into main from sai/OptionsDisplayNames

Jul 7, 2022

Contributor

ssreerama commented Jun 21, 2022

…tributes

This PR provides

OptionsMapTable {key:DisplayName, value: {value, description, propertyName}} to SQL DB Proj and SC
This change helps to remove the duplicate code form the extensions and utilize the direct useful information from the DacFx through the optionsMapTable.
Test fixes and added conditions for optionsMapTable values.


          OptionsMapTable preparing from DacFx description,value,displayname at…

8479ac1

…tributes

ssreerama requested review from Benjin and kisantia

June 21, 2022 17:54

ssreerama mentioned this pull request

Sai/dac options map table #1542

Closed

ssreerama marked this pull request as ready for review

June 21, 2022 18:47

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/DacFx/DacFxServiceTests.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


          Review Comments addressed for options map table changes

92a1448

ssreerama self-assigned this

ssreerama requested a review from kisantia

June 23, 2022 15:41

kisantia approved these changes

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

ssreerama added 2 commits

June 27, 2022 19:49


          Merge branch 'main' into sai/OptionsDisplayNames

c50f031


          OptionsMapTable final changes

63061b5

Contributor

kisantia commented Jun 28, 2022

Are the test failures because this PR needs a new DacFx preview nuget?

Contributor Author

ssreerama commented Jun 28, 2022

Are the test failures because this PR needs a new DacFx preview nuget?

@kisantia , Yes, working on getting the Preview Nuget for the ADS, will updated it today once the Nuget is available. Thanks :)

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved


          Code review comments updated code changes

0b44b57

chlafreniere requested a review from kisantia

June 29, 2022 05:37


          Test fix: Adding missing change while splitting the PR

73dc423

ssreerama requested a review from chlafreniere

June 29, 2022 16:40


          DacFx vBump

d1160ca

github-actions bot commented Jun 29, 2022

As part of updating the dependencies in Packages.props we require that any PRs opened also verify that
they've done the following checks.

Please respond to this comment verifying that you've done the appropriate validation (or explain why it's not necessary) before merging in the PR

Built and tested the change locally to validate that the update doesn't cause any regressions and fixes the issues intended
Tested changes on all major platforms (Windows/Mac/Linux)
Check the source repo for any open issues with the release being updated to (if available)

Benjin previously approved these changes

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/DacFx/DacFxServiceTests.cs Outdated Show resolved Hide resolved

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/DacFx/DacFxServiceTests.cs Outdated Show resolved Hide resolved

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/DacFx/DacFxServiceTests.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

Benjin self-requested a review

June 29, 2022 20:17

Benjin reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

Benjin dismissed their stale review

June 29, 2022 20:29

Open comments

llali reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

kisantia mentioned this pull request

add parse t-sql script handler #1561

Merged

ssreerama added 3 commits

June 30, 2022 13:09


          Merge branch 'main' into sai/OptionsDisplayNames

0258c68


          Reverted to displayName and code updates

13ca064


          final:prop name changed, references updated, tests fixed, comments ad…

ffa0469

…dressed

Charles-Gagnon reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareTestUtils.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

ssreerama added 3 commits

June 30, 2022 18:46


          Merge branch 'main' into sai/OptionsDisplayNames

7af6792


          Code review comments updated for name,exception etc

08a758e


          updates method names

02755fa

ssreerama requested review from Benjin, Charles-Gagnon and llali

July 1, 2022 19:08

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/DacFx/DacFxServiceTests.cs Outdated Show resolved Hide resolved

ssreerama added 2 commits

July 5, 2022 12:12


          Merge branch 'main' into sai/OptionsDisplayNames

d76a289


          property name changes to BooleanOptionsDictionary and comment updates

fad86ef

ssreerama requested a review from kisantia

July 5, 2022 21:53

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareUtils.cs Outdated Show resolved Hide resolved

kisantia reviewed

View reviewed changes

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

Charles-Gagnon approved these changes

View reviewed changes

Contributor

Charles-Gagnon left a comment

There's a lot of stuff here that I'm not the expert on with what properties exist and how they're passed around, but generally not seeing anything big.

I would very strongly suggest adding a README somewhere for the steps to take when new properties need to be added, covering both how the built-in boolean ones are handled and then how to add "custom" props.

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/DacFx/DacFxServiceTests.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

...oft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareServiceOptionsTests.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved

src/Microsoft.SqlTools.ServiceLayer/DacFx/Contracts/DeploymentOptions.cs Outdated Show resolved Hide resolved


          Removed the unused properties, null cases handeled, hardcoded values …

847440a

…are replaced

kisantia approved these changes

View reviewed changes

llali approved these changes

View reviewed changes

ssreerama merged commit 5663ddb into main

ssreerama deleted the sai/OptionsDisplayNames branch

July 7, 2022 21:56

ssreerama mentioned this pull request

Include object types getting from DacFx enum #1553

Closed

nofield pushed a commit that referenced this pull request


          OptionsMapTable preparing from DacFx description,value,displayname at… (

1ebe7d9

#1550)

* OptionsMapTable preparing from DacFx description,value,displayname attributes

* Review Comments addressed for options map table changes

* OptionsMapTable final changes

* Code review comments updated code changes

* Test fix: Adding missing change while splitting the PR

* DacFx vBump

* Reverted to displayName and code updates

* final:prop name changed, references updated, tests fixed, comments addressed

* Code review comments updated for name,exception etc

* updates method names

* property name changes to BooleanOptionsDictionary and comment updates

* Removed the unused properties, null cases handeled, hardcoded values are replaced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kisantia kisantia approved these changes

Charles-Gagnon Charles-Gagnon approved these changes

llali llali approved these changes

chlafreniere Awaiting requested review from chlafreniere

Benjin Awaiting requested review from Benjin

Labels

None yet