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

Allow root as valid UNIX path #811

Merged
merged 1 commit into from
Sep 26, 2017
Merged

Allow root as valid UNIX path #811

merged 1 commit into from
Sep 26, 2017

Conversation

kofrezo
Copy link
Contributor

@kofrezo kofrezo commented Aug 31, 2017

Since the purpose of this validation type is to check if the given parameter is a valid UNIX path it should also cover the root path "slash" since it might be possible that it is specified as a target directory.

@pmcmaw
Copy link
Contributor

pmcmaw commented Sep 14, 2017

Hey @kofrezo I have had a look at your change. I see you have 2 commits for one change, if you could squash the commit and add a test to test the change I will get this merged.

Many Thanks for your contribution, it is much appreciated.

Since the purpose of this validation type is to check if the given parameter is a valid UNIX path it should also cover the root path "slash" since it might be possible that it is specified as a target directory.
@kofrezo
Copy link
Contributor Author

kofrezo commented Sep 20, 2017

Hi @pmcmaw
Sorry for late response I was on vaccation I sqaushed the commits to one so if tests pass I think everything should be fine :-)

@kofrezo
Copy link
Contributor Author

kofrezo commented Sep 22, 2017

@pmcmaw I don't really get why it is failing could you please help me with it.

@pmcmaw pmcmaw merged commit 2b2b387 into puppetlabs:master Sep 26, 2017
@pmcmaw
Copy link
Contributor

pmcmaw commented Sep 26, 2017

@kofrezo There was an issue with a few tests. This has been resolved by @tphoney and now the build is passing. Many Thanks for your contribution. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants