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

Update Fake.Tools.Pickles #2133

Merged
merged 4 commits into from
Oct 12, 2018
Merged

Conversation

magicmonty
Copy link
Contributor

Description

Pickles has gotten some additional command line parameters
I added them and updated the command line generation from a StringBuilder to
Args.toWindowsCommandLine

TODO

  • New (API-)documentation for new features exist
  • tested manually
  • Fake 5 API guideline is honored

@magicmonty magicmonty force-pushed the Pickles branch 4 times, most recently from 3e25f6c to 6ebc433 Compare October 12, 2018 06:55
@magicmonty
Copy link
Contributor Author

@matthid Added unit tests and fixed the resulting bugs ;)

@matthid
Copy link
Member

matthid commented Oct 12, 2018

Tried to fix on the phone but I failed :(

let expectedPath = Path.Combine("pickles", "pickles.exe")
Expect.equal file expectedPath "Expected pickles.exe"

expectedPath, cp.Command.CommandLine
Copy link
Member

Choose a reason for hiding this comment

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

this needs to use the 'args' variable and not 'cp' in order to work with mono environments...

I think '(RawCommand(file, args)).CommandLine' should work

Can you please change the other ones as well, this should make the build green again

@magicmonty
Copy link
Contributor Author

@matthid It seems now to work except for that the check for mono fails, if there are no arguments at all

@matthid
Copy link
Member

matthid commented Oct 12, 2018

Ah yes, thanks for taking care of this!

matthid
matthid previously approved these changes Oct 12, 2018
@magicmonty
Copy link
Contributor Author

@matthid Yay, Travis is now green too ;)

@matthid matthid merged commit 6514d38 into fsprojects:release/next Oct 12, 2018
@matthid
Copy link
Member

matthid commented Oct 12, 2018

Yes thanks a lot :)

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.

2 participants