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

Sai/dac options map table #1542

Closed
wants to merge 21 commits into from
Closed

Sai/dac options map table #1542

wants to merge 21 commits into from

Conversation

ssreerama
Copy link
Contributor

@ssreerama ssreerama commented Jun 16, 2022

This PR fixes

  1. Removed the approach of duplicating the same logic at two extensions for deployment options and Include objec types
  2. Now these options are sending from STS as optionsMapTable-for deployment options, and includeObjectsTable-ObjectType Enum for include objects tab for both SC and SQL DB Project extension.
  3. All options names are coming from the DacFx API from the property_FriendlyName.
  4. Tests are fixed accordingly, as optionsMapTable have the updated values and are now sending back to DacFx

@ssreerama ssreerama requested review from kisantia and Benjin June 20, 2022 19:03
@ssreerama
Copy link
Contributor Author

Tests are failing because if needs new DacFx nuget to get the names, but they all are passing on local(as i used the DacFx new dlls). DacFx PR is out and once its merged will updated the Nuget version here.

@kisantia
Copy link
Contributor

can you please split this into 2 PRs?

  • one PR for changing the option names to use property_FriendlyName
  • one PR for the map changes

@ssreerama
Copy link
Contributor Author

since FriendlyName is being used in Map Table, cannot split it out of maptable, but may be I can try splitting include objects types related changes from map table, I want top use this PR as is for my development and will generate 2 new PRs. Thanks

@ssreerama
Copy link
Contributor Author

ssreerama commented Jun 21, 2022

Separated these changes into 2 PRs:

  1. OptionsMapTable for Deployment options- OptionsMapTable preparing from DacFx description,value,displayname at… #1550
  2. IncludeObjectsTable for Include Object types of options - Include object types getting from DacFx enum #1553

@ssreerama ssreerama removed request for Benjin and kisantia June 23, 2022 15:41
@github-actions
Copy link

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)

@ssreerama ssreerama closed this Jul 13, 2022
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