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

SqlServerRole: Added localization and integration tests #996

Merged
merged 5 commits into from
Jan 7, 2018

Conversation

johlju
Copy link
Member

@johlju johlju commented Jan 5, 2018

Pull Request (PR) description

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

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 johlju changed the title SqlServerRole: added localization and integration tests SqlServerRole: Added localization and integration tests Jan 5, 2018
@johlju johlju added the waiting for author response The pull request is waiting for the author to respond to comments in the pull request. label Jan 5, 2018
@codecov-io
Copy link

codecov-io commented Jan 5, 2018

Codecov Report

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

Impacted file tree graph

@@         Coverage Diff          @@
##            dev   #996    +/-   ##
====================================
+ Coverage    97%    97%   +<1%     
====================================
  Files        32     32            
  Lines      3904   3914    +10     
====================================
+ Hits       3817   3827    +10     
  Misses       87     87

@johlju
Copy link
Member Author

johlju commented Jan 6, 2018

This PR is dependent on PR #998. I will update this PR with more integration tests once PR #998 is merged.

johlju added 2 commits January 6, 2018 16:01
- Added localization ([issue dsccommunity#621](dsccommunity#621)).
- Added an outline of integration tests.
@johlju johlju force-pushed the fix-localization-SqlServerRole branch from fb2057f to e6432d3 Compare January 6, 2018 15:01
@johlju johlju added needs review The pull request needs a code review. and removed waiting for author response The pull request is waiting for the author to respond to comments in the pull request. labels Jan 6, 2018
@johlju
Copy link
Member Author

johlju commented Jan 7, 2018

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


DSCResources/MSFT_SqlServerRole/MSFT_SqlServerRole.psm1, line 133 at r1 (raw file):

$memberToInclude

Should be $memberToExclude


DSCResources/MSFT_SqlServerRole/en-US/MSFT_SqlServerRole.strings.psd1, line 1 at r1 (raw file):

SqlSetup

Should be SqlServerRole


Tests/Integration/MSFT_SqlServerRole.config.ps1, line 123 at r1 (raw file):

        Forces members in an existing server role to be exactly these members,
        no more, no less.
        Role1 started out with one member, but will end up containing only two

Suggest we remove row 3 and 4 in the comment.


Comments from Reviewable

@johlju
Copy link
Member Author

johlju commented Jan 7, 2018

Reviewed 1 of 3 files at r2.
Review status: 8 of 10 files reviewed at latest revision, 2 unresolved discussions.


DSCResources/MSFT_SqlServerRole/MSFT_SqlServerRole.psm1, line 133 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$memberToInclude

Should be $memberToExclude

Done.


DSCResources/MSFT_SqlServerRole/en-US/MSFT_SqlServerRole.strings.psd1, line 1 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

SqlSetup

Should be SqlServerRole

Done.


Comments from Reviewable

@johlju
Copy link
Member Author

johlju commented Jan 7, 2018

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


Comments from Reviewable

@johlju
Copy link
Member Author

johlju commented Jan 7, 2018

:lgtm:


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


Comments from Reviewable

@johlju johlju merged commit f8449a4 into dsccommunity:dev Jan 7, 2018
@johlju johlju removed the needs review The pull request needs a code review. label Jan 7, 2018
@johlju johlju deleted the fix-localization-SqlServerRole branch March 12, 2018 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants