-
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
SmbShare: Bugfix to enable change of the path when the share already exists. #286
Conversation
85b5910
to
dbb8f9f
Compare
Codecov Report
@@ Coverage Diff @@
## dev #286 +/- ##
====================================
+ Coverage 88% 88% +<1%
====================================
Files 14 14
Lines 1469 1478 +9
====================================
+ Hits 1294 1302 +8
- Misses 175 176 +1 |
Hi @PlagueHO, on another branch I have a fix for #284 (https://github.com/danielboth/ComputerManagementDsc/tree/SmbShare-ScopeName-support). That one builds on the features I've added in this branch, would you like to review those in one go, or that we first merge this one? |
Hi @danielboth - can you resolve the conflicts on this one? My feeling is to complete this PR first (as it is a bug fix) and then we'll get the feature one through. |
Good, I got the same feeling (the reason I splitted them to start with), so let's get this one through first. Just resolved the merge conflict. |
Closing/Reopening to kick CI again. |
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 8 files at r1, 1 of 2 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @danielboth)
DSCResources/MSFT_SmbShare/MSFT_SmbShare.schema.mof, line 18 at r3 (raw file):
[Write, Description("Specifies which accounts is granted read permission to access the SMB share.")] String ReadAccess[]; [Write, Description("Specifies if the SMB share should be added or removed."), ValueMap{"Present","Absent"}, Values{"Present","Absent"}] String Ensure; [Write, Description("Specifies if the SMB share is allowed to be dropped and recreated.")] Boolean Force;
This text should match the value in the PSM1. Can you add the part in the parenthesis from the parenthesis?
Examples/Resources/SmbShare/4-SmbShare_RecreateShare_Config.ps1, line 3 at r3 (raw file):
<#PSScriptInfo .VERSION 1.0.0 .GUID d0847694-6a83-4f5b-bf6f-30cb078033bc
Can you generate a new GUID? This one looks like it's already used by one of the other examples.
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: 6 of 8 files reviewed, 2 unresolved discussions (waiting on @PlagueHO)
DSCResources/MSFT_SmbShare/MSFT_SmbShare.schema.mof, line 18 at r3 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
This text should match the value in the PSM1. Can you add the part in the parenthesis from the parenthesis?
Done.
Examples/Resources/SmbShare/4-SmbShare_RecreateShare_Config.ps1, line 3 at r3 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Can you generate a new GUID? This one looks like it's already used by one of the other examples.
Is this GUID used? Can you explain why it's important for this one to be unique?
Anyway, updated now, just like to know the why.
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 2 of 2 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved
Examples/Resources/SmbShare/4-SmbShare_RecreateShare_Config.ps1, line 3 at r3 (raw file):
Previously, danielboth (Daniël) wrote…
Is this GUID used? Can you explain why it's important for this one to be unique?
Anyway, updated now, just like to know the why.
The Guid represents the unique id of the script (not the filename as you might expect). The intent was that this was how PS Gallery would uniquely identify a script. Whether it does or not remains to be seen (it may just use the filename) - but it is still best to follow the intent of the field.
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: complete! all files reviewed, all discussions resolved
Pull Request (PR) description
This changes enables the SmbShare resource to change the path on an existing share. To ensure a user does this consciously, a parameter Force is added to the resource. Force needs to be true for the resource to drop and recreate the share on the new path.
If the user did not set Force to true, a warning will be displayed stating the resource cannot get into desired state without setting Force to true.
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.
This change is