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: xSQLServerHelper: supporting SqlServer module #508

Merged
merged 3 commits into from
Apr 19, 2017

Conversation

johlju
Copy link
Member

@johlju johlju commented Apr 17, 2017

Pull Request (PR) description

  • Changes to xSQLServerHelper module
  • Changes to xSQLServerScript
    • Updated tests for this resource, because they failed when Import-SQLPSModule was updated.
  • Changes to ISSUE_TEMPLATE
    • Added new control question to ISSUE_TEMPLATE to know which module the user is using when submitting an issue.

This Pull Request (PR) fixes the following issues:
Fixes #91

Task list:

  • Change details added to Unreleased section of CHANGELOG.md?
  • Added/updated documentation, comment-based help and descriptions in .schema.mof files where appropriate?
  • Examples appropriately updated?
  • New/changed code adheres to Style Guidelines?
  • Unit and (optional) Integration tests created/updated where possible?

This change is Reviewable

johlju added 2 commits April 17, 2017 15:20
Changed helper function Import-SQLPSModule to support SqlServer module (issue dsccommunity#91). The SqlServer module is the preferred module so if it is found it will be used, and if not found an attempt will be done to load SQLPS module instead.
Added new control question to ISSUE_TEMPLATE to know which module the user is using when submitting an issue.
Tests failed when Import-SQLPSModule helper function changed.
@johlju johlju added the needs review The pull request needs a code review. label Apr 17, 2017
@codecov-io
Copy link

codecov-io commented Apr 17, 2017

Codecov Report

Merging #508 into dev will increase coverage by <1%.
The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff          @@
##            dev   #508    +/-   ##
====================================
+ Coverage    75%    75%   +<1%     
====================================
  Files        32     32            
  Lines      3445   3449     +4     
====================================
+ Hits       2615   2621     +6     
+ Misses      830    828     -2

@johlju
Copy link
Member Author

johlju commented Apr 18, 2017

Issue #91 has a comment about testing stubs for both SQLPS and SqlServer module. I will open a separate issue for that because that needs some planing. The tests take a long time to run, so running them twice might not be the best option since there is currently a limit on 1 hour runtime for the build worker.

Update: Issue #509 submitted for this.

@johlju
Copy link
Member Author

johlju commented Apr 19, 2017

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


CHANGELOG.md, line 115 at r1 (raw file):

- Changes to xSQLServerHelper module
  - Removing helper function Get-SQLAlwaysOnEndpoint becuase there is no resource using it any longer.
  - Changed helper function Import-SQLPSModule to support SqlServer module (issue #91). The SqlServer module is the preferred module so if it is found it will be used, and if not found an attempt will be done to load SQLPS module instead.

Make this a breaking change, we are changing the previous behaviour. Change description in CHANGELOG, PR Title and och Issue Title to prefix with 'BREAKING CHANGE:'


.github/ISSUE_TEMPLATE.md, line 15 at r1 (raw file):

**Version of the Operating System, SQL Server and PowerShell the DSC Target Node is running:**

**What module (SqlServer or SQLPS) and which version of the module is the node running:**

Change 'node' to 'DSC Target Node'


Comments from Reviewable

@johlju johlju changed the title xSQLServerHelper: supporting SqlServer module BREAKING CHANGE: xSQLServerHelper: supporting SqlServer module Apr 19, 2017
@johlju johlju added waiting for author response The pull request is waiting for the author to respond to comments in the pull request. and removed needs review The pull request needs a code review. labels Apr 19, 2017
@johlju
Copy link
Member Author

johlju commented Apr 19, 2017

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


CHANGELOG.md, line 115 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Make this a breaking change, we are changing the previous behaviour. Change description in CHANGELOG, PR Title and och Issue Title to prefix with 'BREAKING CHANGE:'

Done.


.github/ISSUE_TEMPLATE.md, line 15 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Change 'node' to 'DSC Target Node'

Done.


Comments from Reviewable

@johlju
Copy link
Member Author

johlju commented Apr 19, 2017

:LGTM:


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@johlju johlju removed the waiting for author response The pull request is waiting for the author to respond to comments in the pull request. label Apr 19, 2017
@johlju johlju merged commit 21d2f00 into dsccommunity:dev Apr 19, 2017
@johlju johlju deleted the add-support-for-sqlserver-module branch April 19, 2017 20:39
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.

BREAKING CHANGE: Check for SQLPS & SQLServer modules required
3 participants