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

use ImplicitZero instead of Return unit #773

Merged
merged 2 commits into from
Jan 20, 2016

Conversation

mexx
Copy link
Contributor

@mexx mexx commented Dec 7, 2015

Motivation: fsprojects/FSharp.Control.AsyncSeq#38

UserVoice

This PR don't have any new test, any guidance where to find the tests for CEs and to put new for the change is welcome.

@KevinRansom
Copy link
Member

Perhaps here: tests\fsharp\core\comprehensions\test.fsx ?

@mexx
Copy link
Contributor Author

mexx commented Dec 15, 2015

I've added the tests. Comments welcome.


module SideEffectListTests =
#if Portable
let printfn s = printfn "%s" s
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you copy/paste from near tests (good), but maybe this is good opportunity to cleanup this duplication, adding the printfn function at top level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@enricosada maybe, or better to get rid of the root cause. I mean why isn't it simply working for Portable too?

@dsyme
Copy link
Contributor

dsyme commented Dec 15, 2015

@KevinRansom KevinRansom merged commit 130c86b into dotnet:master Jan 20, 2016
@dsyme
Copy link
Contributor

dsyme commented Jan 21, 2016

Thanks for the contribution @mexx !!

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.

5 participants