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 #33 #34

Merged
merged 5 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 xNetworking, xStorage and xCertificate 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 #33


This change is Reviewable

…urce kit modules.

- Prevented ResourceHelper and Common module cmdlets from being exported to resolve
  conflicts with other resource modules.
@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 #34 into dev will increase coverage by <1%.
The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff          @@
##            dev    #34    +/-   ##
====================================
+ Coverage    96%    96%   +<1%     
====================================
  Files         7      7            
  Lines      1013   1020     +7     
====================================
+ Hits        978    985     +7     
  Misses       35     35

@johlju
Copy link
Member

johlju commented Jun 1, 2017

@PlagueHO Did you branch this from the issue-31 working branch? Looks like you got those changes in here as well (?). Could you rebase against dev?

@johlju
Copy link
Member

johlju commented Jun 1, 2017

And the same comment as in xStorage

@PlagueHO
Copy link
Member Author

PlagueHO commented Jun 1, 2017

Doh! @johlju - you're right - I did. It was getting really late and I didn't check the branch I was in when I created the new one. Fixed now. Also changed the "Should not throw" to "Should Not Throw"

@johlju
Copy link
Member

johlju commented Jun 1, 2017

I figured ;) No worries :)

@johlju
Copy link
Member

johlju commented Jun 1, 2017

:lgtm:


Reviewed 9 of 10 files at r1, 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 @johlju !

@PlagueHO PlagueHO merged commit f07eb8b 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-33 branch June 1, 2017 20:38
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