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

Disable Quoting of process filename on unix #560

Merged
merged 1 commit into from
Dec 9, 2015

Conversation

devlead
Copy link
Member

@devlead devlead commented Dec 7, 2015

Fixes #559

@patriksvensson
Copy link
Member

Great! 👍

Can we also add unit tests for this as well here?

@@ -48,13 +48,16 @@ public IProcess Start(FilePath filePath, ProcessSettings settings)
throw new ArgumentNullException("settings");
}

// Get the fileName
var fileName = _environment.IsUnix() ? filePath.FullPath : filePath.FullPath.Quote();
Copy link
Member

Choose a reason for hiding this comment

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

This variable name confused me a little since it's the whole path that this contains and not just the file name. Maybe we can change this to fullFileName or something that shows that this is the full path to the file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same name as ProcessStartInfo constructor parameter. Suggest we keep.

@patriksvensson
Copy link
Member

@devlead I read the code again and no tests are necessary for this one. It would require an integration test so let's just verify that it works for now.

@gep13 gep13 merged commit 025a2c7 into cake-build:main Dec 9, 2015
@devlead devlead deleted the hotfix/0.6.4 branch March 20, 2016 19:03
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.

3 participants