Skip to content
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 validation of CollectionName length 1-15 characters and tests #15

Merged
merged 3 commits into from
Feb 6, 2017

Conversation

JimPriestley
Copy link
Contributor

@JimPriestley JimPriestley commented Jan 13, 2017

Resubmission of Issue 10

  • Added validation of CollectionName parameter 1-15 characters
  • Added Tests for CollectionName length validation
  • Updated README.md to add documentation to unreleased section, and fixed bad apostrophe
  • Added NewLine to end of xRemoteDesktopSessionHostCommon.psm1

This change is Reviewable

@msftclas
Copy link

Hi @JimPriestley, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Jim Priestley). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@kwirkykat
Copy link
Contributor

@TravisEz13 Changes from #13 are now here.

@kwirkykat kwirkykat added the needs review The pull request needs a code review. label Jan 19, 2017
Export-ModuleMember -Function *-TargetResource

Import-Module -Name "$PSScriptRoot\..\..\xRemoteDesktopSessionHostCommon.psm1"
if (!(Test-xRemoteDesktopSessionHostOsRequirement)) { Throw "The minimum OS requirement was not met."}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import is not a good place to throw an error. It will basically be hidden. This should be turned into a function and called at the beginning of test set and get

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TravisEz13 You wrote that code and committed it on Feb 23, 2016. It is not part of the code I wrote. It is also not a test, it appears to be part of the functional code to block the resource being deployed on an OS that does not support the feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, we can just file an issue for it.

This change involves a formatting change. Can you submit the formatting change separately from the code changes. I cannot tell what changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filed #16

@TravisEz13
Copy link
Contributor

Reviewed 8 of 8 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@TravisEz13 TravisEz13 changed the title Issue10 fix - Added validation of CollectionName length 1-15 characters and tests Add validation of CollectionName length 1-15 characters and tests Feb 6, 2017
@TravisEz13 TravisEz13 merged commit fbdf74e into dsccommunity:dev Feb 6, 2017
@TravisEz13 TravisEz13 removed the needs review The pull request needs a code review. label Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants