-
Notifications
You must be signed in to change notification settings - Fork 82
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
Convert to use DSCResource.Common Module #334
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 23 of 23 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved
source/DSCResources/DSC_Computer/DSC_Computer.psm1, line 16 at r1 (raw file):
-DefaultUICulture 'en-US'
I haven't tried it myself yet, but a recent change to the function made this the default, so just calling Get-LocalizedData
should work. But this is good as-is.
source/Modules/ComputerManagementDsc.Common/ComputerManagementDsc.Common.psm1, line 634 at r1 (raw file):
'Get-RegistryPropertyValue'
This is a one we use in SqlServerDsc too. We should add it to Common eventually. Good as-is. Non-blocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 7 of 23 files reviewed, all discussions resolved (waiting on @johlju)
source/DSCResources/DSC_Computer/DSC_Computer.psm1, line 16 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
-DefaultUICulture 'en-US'
I haven't tried it myself yet, but a recent change to the function made this the default, so just calling
Get-LocalizedData
should work. But this is good as-is.
I gave it a try without the -DefaultUICulture
- unfortunately it looks like there is a bug - it doesn't find the data file:
I'll submit a fix to DscResource.Common.
Would be great with a fix! |
Pull Request (PR) description
ModuleBuilder
module v1.7.0by changing
CopyDirectories
toCopyPaths
- Fixes Issue #332.This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
@johlju - would you mind reviewing when you get time?
This change is