-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add public commands Get-PSModulePath
and Get-EnvironmentVariable
#104
Add public commands Get-PSModulePath
and Get-EnvironmentVariable
#104
Conversation
Codecov Report
@@ Coverage Diff @@
## main #104 +/- ##
===================================
- Coverage 92% 92% -1%
===================================
Files 35 37 +2
Lines 628 645 +17
===================================
+ Hits 579 594 +15
- Misses 49 51 +2
|
Get-PSModulePath
and Get-EnvironmentVariable
Get-PSModulePath
and Get-EnvironmentVariable
b357516
to
15d0ce2
Compare
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 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @johlju)
README.md
line 723 at r1 (raw file):
Returns the environment variable PSModulePath from the specified target. If more than one target are provided only the unique paths will be
Could this be changed to:
If more than one target is provided the return will contain all the concatenation of all unique paths from the targets.
source/Public/Get-PSModulePath.ps1
line 8 at r1 (raw file):
.DESCRIPTION Returns the environment variable PSModulePath from the specified target. If more than one target are provided only the unique paths will be
See previous comment on clarification of behavior.
source/Public/Get-PSModulePath.ps1
line 41 at r1 (raw file):
) $modulePathSession = $null
nit: I don't think (I couldn't find) a style guideline on this, but you could use (although not sure it's the most clear syntax). Will leave it up to you:
Suggestion:
$modulePathSession = $modulePathUser = $modulePathMachine = $null
tests/Unit/Public/Get-PSModulePath.Tests.ps1
line 86 at r1 (raw file):
} }
Nit: remove blank line.
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: 3 of 6 files reviewed, 2 unresolved discussions (waiting on @PlagueHO)
README.md
line 723 at r1 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Could this be changed to:
If more than one target is provided the return will contain all the concatenation of all unique paths from the targets.
Done.
source/Public/Get-PSModulePath.ps1
line 8 at r1 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
See previous comment on clarification of behavior.
Done.
source/Public/Get-PSModulePath.ps1
line 41 at r1 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
nit: I don't think (I couldn't find) a style guideline on this, but you could use (although not sure it's the most clear syntax). Will leave it up to you:
Done. We can do it this way too. Changed. 🙂
tests/Unit/Public/Get-PSModulePath.Tests.ps1
line 86 at r1 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Nit: remove blank line.
Done.
@PlagueHO thanks for the review! Hopefully it good to be sign-off on now. 🙂 |
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 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johlju)
Pull Request (PR) description
Get-EnvironmentVariable
- Get a specific environment variable from aspecific environment variable target.
Get-PSModulePath
- Get the the PSModulePath from one or more environmentvariable targets - Issue #103
This Pull Request (PR) fixes the following issues
Get-PSModulePath
: New command proposal #103Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
DSC Community Testing Guidelines.
This change is