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

Release 5.9 #2137

Merged
merged 71 commits into from
Oct 13, 2018
Merged

Release 5.9 #2137

merged 71 commits into from
Oct 13, 2018

Conversation

matthid
Copy link
Member

@matthid matthid commented Oct 10, 2018

[
yield parameters.SubCommand |> string

if not (isNull parameters.ProjectFile) then
Copy link
Member Author

Choose a reason for hiding this comment

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

@magicmonty I noticed while writing the test that null leads to an ugly exception. Is this parameter required? Can we give a good default>

Copy link
Contributor

@magicmonty magicmonty Oct 10, 2018

Choose a reason for hiding this comment

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

@matthid
Which one gives an exception? The SubCommand or the ProjectFile?
Both are needed. For the SubCommand the default should be "generateall"
The Project file is needed and thus must not be null. This is the reason I added options on the SpecFlowNext variant

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok if it is needed can you please remove the nullcheck I added and make sure that user gets a proper error message if the parameter is not given?

Copy link
Contributor

Choose a reason for hiding this comment

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

@matthid
I removed the null check and adjusted the API a bit. The ProjectFile now needs to be provided on "run". I have opened a new PR for this, as I cannot push to this branch

[<Tests>]
let tests =
testList "Fake.DotNet.Testing.SpecFlow.Tests" [
testCase "Test that new argument generation works" <| fun _ ->
Copy link
Member Author

Choose a reason for hiding this comment

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

@magicmonty Can you take a look at this test (if it is reasonably) and maybe add some to the other PRs as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we need to run the specflow exe with mono when not on windows?

Copy link
Contributor

@magicmonty magicmonty Oct 10, 2018

Choose a reason for hiding this comment

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

@matthid

Do we need to run the specflow exe with mono when not on windows?

I don't know. I suppose so.

Copy link
Contributor

Choose a reason for hiding this comment

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

@magicmonty Can you take a look at this test (if it is reasonably) and maybe add some to the other PRs as well?

Sure, I have a look

Copy link
Contributor

Choose a reason for hiding this comment

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

@matthid
I have updated the Tests in a new PR. However it seems I do something wrong with the Mono invocation. Can you give me a hint there?

@matthid matthid changed the title Next Release Release 5.9 Oct 12, 2018
matthid and others added 11 commits October 13, 2018 01:09
* Add message to the build state update

* Update Fake.Tools.Pickles and added Unit tests

* Fix Tests

* Fix ArgumentHelper.checkIfMono

* Made checkIfMono a bit more elegant

* Correct publish artifact

* Add `CreateProcess.disableTraceCommand` and use it to hide `appveyor.exe` calls.

* print all stack traces in verbose mode, references #2136

* next time we can fix #2136

* add BlackFox.Fake.BuildTask to docs /cc @vbfox

* Docs & cleanup

* more docs

* documentation
Seperate runInternal & handle error
Helper for update Build-Status
@matthid matthid merged commit a53f9ed into master Oct 13, 2018
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