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

Prevent ResourceHelper and Common cmdlets from being exported - Fixes #213 #214

Merged
merged 7 commits into from
Jun 1, 2017
Merged

Conversation

PlagueHO
Copy link
Member

@PlagueHO PlagueHO commented Jun 1, 2017

This resolves a high priority issue with newer versions of xNetworking, xCertificate, xStorage and xDFS conflicting with each other because the ResourceHelper cmdlets are being exported due to the way each resource imported them (using the PSD1 nested modules array).

This should be resolved in xCertificate, xStorage and xDFS as well to ensure no other conflicts.

This also contains a new integration test that will check for any further conflicts between resource modules. At some point it might be possible to move this test into the DSCResource.Tests common tests to provide more comprehensive conflict prevention. However, because the conflicts will potentially be between three different modules the number of modules compared will have to go in over time.

This fixes #213


This change is Reviewable

@PlagueHO PlagueHO added high priority The issue or PR should be resolved first. It is of less priority than the label 'Blocking Release'. needs review The pull request needs a code review. labels Jun 1, 2017
@codecov-io
Copy link

codecov-io commented Jun 1, 2017

Codecov Report

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

Impacted file tree graph

@@         Coverage Diff          @@
##            dev   #214    +/-   ##
====================================
+ Coverage    96%    96%   +<1%     
====================================
  Files        17     17            
  Lines      1222   1273    +51     
====================================
+ Hits       1174   1225    +51     
  Misses       48     48

@johlju
Copy link
Member

johlju commented Jun 1, 2017

Reviewed 20 of 20 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Tests/Integration/ModuleConflict.Tests.ps1, line 37 at r1 (raw file):

                {
                    Install-Module -Name $moduleToTest -ErrorAction Stop
                } | Should not throw

Same comment as in xStorage


Comments from Reviewable

@PlagueHO
Copy link
Member Author

PlagueHO commented Jun 1, 2017

This too could use a global search replace to correct the style, but I'll do this in a separate issue.


Review status: 19 of 20 files reviewed at latest revision, 1 unresolved discussion.


Tests/Integration/ModuleConflict.Tests.ps1, line 37 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Same comment as in xStorage

Done.


Comments from Reviewable

@johlju
Copy link
Member

johlju commented Jun 1, 2017

:lgtm:


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


Comments from Reviewable

@PlagueHO
Copy link
Member Author

PlagueHO commented Jun 1, 2017

Thanks again so much for your help @johlju

@PlagueHO PlagueHO merged commit 17643c2 into dsccommunity:dev Jun 1, 2017
@vors vors removed high priority The issue or PR should be resolved first. It is of less priority than the label 'Blocking Release'. needs review The pull request needs a code review. labels Jun 1, 2017
@PlagueHO PlagueHO deleted the Issue-213 branch June 1, 2017 20:34
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: Get-LocalizedData command
6 participants