Removed unnecessary entries #55
Conversation
Codecov Report
@@ Coverage Diff @@
## master PowerShell/PowerShell#55 +/- ##
====================================
Coverage 35% 35%
====================================
Files 5 5
Lines 445 445
====================================
Hits 160 160
Misses 285 285
|
There was a problem hiding this comment.
Reviewed 1 of 3 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @raandree)
build.yaml line 37 at r2 (raw file):
- Create_changelog_release_output - Generate_Conceptual_Help #- Generate_Wiki_Content
Why do we have to remove this task?
Code quote:
#- Generate_Wiki_Contentbuild.yaml line 54 at r2 (raw file):
- publish_module_to_gallery - Publish_Release_To_GitHub #- Publish_GitHub_Wiki_Content
Why do we have to remove this task?
Code quote:
#- Publish_GitHub_Wiki_Contentbuild.yaml line 67 at r2 (raw file):
- Modules/DscResource.Common Script: - tests/Unit
We usually have just the unit tests here so that ./build.ps1 -Task test does not make permanent changes to a contributors dev machine. Integration tests will run if this is just set to tests. We usually make a user opt-in to run them by running ./build.ps1 -Task test -PesterScript ./tests/Integration. Are the integration tests in the repo such that they do not make changes to a contributors machine?
Code quote:
- tests/Unitsource/JeaDsc.psd1 line 29 at r2 (raw file):
VariablesToExport = @() AliasesToExport = @()
Do we need to remove AliasesToExport too? It would be good to have that one working as ModuleBuilder will generate that one automatically if there are aliases on on public commands.
Code quote:
AliasesToExport = @()
There was a problem hiding this comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @raandree)
source/JeaDsc.psd1 line 29 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Do we need to remove
AliasesToExporttoo? It would be good to have that one working as ModuleBuilder will generate that one automatically if there are aliases on on public commands.
In my tests (thanks to @raandree for steps to reproduce) I saw that it is enough to remove CmdletsToExport. I suggest just removing that property, and add a unit test e.g. tests/QA/check_module_manifest.tests.ps1 and add the folder tests/QA as a folder to run for task test.
- JeaDsc
- Renamed class files adding a prefix on each file so the task `Generate_Wiki_Content`
works (reported issue dsccommunity/DscResource.DocGenerator#132).
…JeaDsc into fix/psd1Properties
There was a problem hiding this comment.
Thanks, done all the changes
Reviewable status: 1 of 10 files reviewed, 4 unresolved discussions (waiting on @johlju)
build.yaml line 37 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Why do we have to remove this task?
Done, thanks for your last fix.
build.yaml line 54 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Why do we have to remove this task?
Done, thanks for your last fix.
build.yaml line 67 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We usually have just the unit tests here so that
./build.ps1 -Task testdoes not make permanent changes to a contributors dev machine. Integration tests will run if this is just set totests. We usually make a user opt-in to run them by running./build.ps1 -Task test -PesterScript ./tests/Integration. Are the integration tests in the repo such that they do not make changes to a contributors machine?
I see, makes sense. Done.
source/JeaDsc.psd1 line 29 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
In my tests (thanks to @raandree for steps to reproduce) I saw that it is enough to remove
CmdletsToExport. I suggest just removing that property, and add a unit test e.g.tests/QA/check_module_manifest.tests.ps1and add the foldertests/QAas a folder to run for tasktest.
Done.
|
@johlju, do you think this is ready to be merged? |
There was a problem hiding this comment.
Reviewed 5 of 8 files at r3, 4 of 4 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @raandree)
tests/QA/Module.Tests.ps1 line 8 at r4 (raw file):
It "Does not contain key 'CmdletsToExport'" { $psd1.ContainsKey('CmdletsToExport') | Should Be $false
We should use Should -BeFalse. But can be fixed in a new PR that converts the tests to Pester 5.
tests/QA/Module.Tests.ps1 line 12 at r4 (raw file):
It "Does contain key 'DscResourcesToExport'" { $psd1.ContainsKey('DscResourcesToExport') | Should Be $true
We should use Should -BeTrue. But can be fixed in a new PR that converts the tests to Pester 5.
Pull Request (PR) description
In PowerShell 7, DSC resources published by this module cannot be found due to this bug: Get-DscResource doesn't identify the class based DSC resource PowerShell/PSDesiredStateConfiguration#117. This PR removes the unnecessary entries in the module manifest.
This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed and how that affects users (if applicable), and
reference the issue being resolved (if applicable).
This change is