-
Notifications
You must be signed in to change notification settings - Fork 42
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
adding some fixes #428
base: main
Are you sure you want to change the base?
adding some fixes #428
Conversation
Codecov Report
@@ Coverage Diff @@
## main #428 +/- ##
===================================
Coverage 81% 81%
===================================
Files 44 44
Lines 2319 2319
===================================
Hits 1899 1899
Misses 420 420 |
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.
Asking @johlju to review and assess the impact.
Actually didn't realize that this was different for some modules in DSC Community. It will fail on for example ExchangeDsc but for example SqlServerDsc and others already have this change. Simple fix by adding the correct config. The impact will be som maintaining work to unblock PRs. But this change is good and needed.
Looks good if the RequiredModules file do not need updating.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gaelcolas)
a discussion (no related file):
Might make sure the module is added here too. Maybe the if statement need to revised too?
Sampler/Sampler/Templates/Build/RequiredModules.psd1.template
Lines 39 to 46 in e9155b8
If ($PLASTER_PARAM_ModuleType -in @('dsccommunity','CompleteSample') -or $PLASTER_PARAM_Features -Contains ('All') -or $PLASTER_PARAM_Features -Contains ('DSCResources')) { | |
@" | |
'DscResource.Common' = 'latest' | |
'DscResource.Test' = 'latest' | |
'DscResource.AnalyzerRules' = 'latest' | |
xDscResourceDesigner = 'latest' | |
'DscResource.DocGenerator' = 'latest' | |
"@ |
Code quote (from Sampler/Templates/Build/build.yaml.template):
DscResource.Test:
- 'Task.*'
build.yaml
line 177 at r1 (raw file):
GitHubFilesToAdd: - 'CHANGELOG.md' ReleaseAssets:
Curious. Is this used somewhere by something?
Code quote:
ReleaseAssets:
We probably should raise an issue in the affected repos before merging/releasing then. Regarding the For instance in the Chocolatey module I add the Azure Automanage Machine Configuration packages/zips: |
Cool. Will see if I can implement it for SqlServerDsc (and then add it to Sampler). |
I will slowly fix the most used repos so we can get this merged. |
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: all files reviewed, 3 unresolved discussions (waiting on @gaelcolas)
Sampler/Templates/Build/build.yaml.template
line 255 at r1 (raw file):
@" DscTest:
Must have the key MainGitBranch:
set to <%= ${PLASTER_PARAM_MainGitBranch} %>
under the key DscTest
.
After trying to update SharePointDsc and failed - concluded the task in Sampler cannot be removed because the new task in DscResource.Test only support Pester 5 currently. The task in Sampler uses the v4 tests in DscResource.Test. The DscResource.Test command Old task calls the command.
The new task calls |
This is to much work to do now. :/ I put it back on the back-burner... |
Fixing template and deprecating an old task we probably shouldn't be using anymore...
Asking @johlju to review and assess the impact.
This change is