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

BREAKING CHANGE: ReverseDSC Native Support #1434

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

NikCharlebois
Copy link

@NikCharlebois NikCharlebois commented Sep 27, 2019

Pull Request (PR) description

This PR introduces native ReverseDSC support for SQLServerDSC. It exposes a new Export-SQLServerConfiguration cmdlet that Reverse Engineers existing SQL Server environment and generates a resulting DSC script that is a full-fidelity representation of the existing settings on the server.

This Pull Request (PR) fixes the following issues

N/A

Task list

  • Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

@codecov-io
Copy link

codecov-io commented Sep 27, 2019

Codecov Report

Merging #1434 into dev will decrease coverage by 7%.
The diff coverage is 36%.

Impacted file tree graph

@@          Coverage Diff          @@
##            dev   #1434    +/-   ##
=====================================
- Coverage    98%     90%    -8%     
=====================================
  Files        38      39     +1     
  Lines      5603    6026   +423     
=====================================
- Hits       5497    5444    -53     
- Misses      106     582   +476

@NikCharlebois
Copy link
Author

Will work on adding Unit Tests later today

@johlju johlju added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Sep 27, 2019
Trying to see how test engine works, since this is my first commit to this repo.
@NikCharlebois
Copy link
Author

@johlju wait before review. Trying to familiarize myself with the Unit Tests setup so I might submit a few extra commits. You can have a look at the general structure of the PR though to get an idea of what's coming your way :)

@gaelcolas
Copy link
Member

Hey @NikCharlebois, that's a huge effort you put in, but I wouldn't be too happy putting that much code in the DSC resources, even more that the feature is not (some could argue 'yet') part of the DSC technology.
Could you maybe try to have minimal code within the DSC resource's psm1s, and have the execution done in another module (i.e. under a .\modules folder).

@johlju
Copy link
Member

johlju commented Oct 24, 2019

There are a lot of duplicated code introduced with this change, I also want to look over this to see that it actually the correct way of doing this. Please continue resolving comments on this change, but I will add an 'on hold' label for the time-being so we have chance to discuss this.

I have offline conversation with @NikCharlebois about this, but I have not have time to get back to that yet.

@johlju johlju added the on hold The issue or pull request has been put on hold by a maintainer. label Oct 24, 2019
@johlju johlju changed the base branch from dev to master January 1, 2020 20:59
@johlju
Copy link
Member

johlju commented Jan 3, 2020

The new CI pipeline has merged. This changed the folder structure, and also removed the dev branch. Please rebase against the master branch, and make sure to get your changes into the the file in the new location (under source folder). 😃

Read more here about the new coding workflow:
https://dsccommunity.org/guidelines/contributing/#understand-the-coding-workflow
https://dsccommunity.org/guidelines/testing-guidelines/#running-tests
You can also use Invoke-Pester once you run build.ps1 -Task build (not documented yet)
https://dsccommunity.org/guidelines/contributing/#attach-your-fork-to-a-free-azure-devops-organization

@kewalaka
Copy link

Hello @NikCharlebois, I see this is the same approach used in Microsoft365Dsc too. In the early days of ReverseDSC this was done as an external "orchestrator" module, such as https://github.com/Microsoft/SQLServerDSC.Reverse, but I see that hasn't been so active recently, did you run into a problem with this approach?

@johlju johlju changed the base branch from master to main January 8, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold The issue or pull request has been put on hold by a maintainer. waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants