Adding uncontentious methods from RFC FS-1004 #1350

Merged
merged 8 commits into from Jul 22, 2016

Conversation

Projects
None yet
4 participants
@wallymathieu
Contributor

wallymathieu commented Jul 14, 2016

https://github.com/fsharp/FSharpLangDesign/blob/master/RFCs/FS-1004-result-type.md

bind : ('T -> Result<'U, 'TError>) -> Result<'T, 'TError> -> Result<'U, 'TError>
map : ('T -> 'U) -> Result<'T, 'TError> -> Result<'U, 'TError>
mapError : ('TError -> 'U) -> Result<'T, 'TError> -> Result<'T, 'U>

The implementation is heavily inspired by how the Option module is structured.

I've added the files in the collections meta folder, since that's where option.fsi and option.fs live (in my mind is somewhat similar).

@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Jul 14, 2016

Hi @wallymathieu, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

Hi @wallymathieu, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

wallymathieu added some commits Jul 14, 2016

@wallymathieu wallymathieu changed the title from [WIP] Adding uncontentious methods from RFC FS-1004 to Adding uncontentious methods from RFC FS-1004 Jul 15, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jul 15, 2016

Contributor

This looks good, I'm ok with this addition to the RFC for this release.

Please add 3-4 unit tests to the FSharp.Core unittests?

Contributor

dsyme commented Jul 15, 2016

This looks good, I'm ok with this addition to the RFC for this release.

Please add 3-4 unit tests to the FSharp.Core unittests?

wallymathieu added some commits Jul 15, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jul 16, 2016

Contributor

AppVeyor lists this failure

[00:33:41] 1) Failed : FSharp-Tests-Core+Members+Incremental.incremental(FSI_FILE)
[00:33:41] Error running command 'C:\projects\visualfsharp-3dtit\tests\..\release\net40\bin\fsiAnyCPU.exe' with args ' -r:System.Core.dll --nowarn:20 --define:INTERACTIVE --maxerrors:1 --abortonerror test.fsx' in directory 'C:\projects\visualfsharp-3dtit\tests\fsharp\core\members\incremental'. ERRORLEVEL -1073740771 ERRORLEVEL -1073740771
[00:33:41] at NUnitConf.checkTestResult(Result`2 result) in C:\projects\visualfsharp-3dtit\tests\fsharp\nunitConf.fs:line 14

Not sure why, it looks unrelated to your changes.

Contributor

dsyme commented Jul 16, 2016

AppVeyor lists this failure

[00:33:41] 1) Failed : FSharp-Tests-Core+Members+Incremental.incremental(FSI_FILE)
[00:33:41] Error running command 'C:\projects\visualfsharp-3dtit\tests\..\release\net40\bin\fsiAnyCPU.exe' with args ' -r:System.Core.dll --nowarn:20 --define:INTERACTIVE --maxerrors:1 --abortonerror test.fsx' in directory 'C:\projects\visualfsharp-3dtit\tests\fsharp\core\members\incremental'. ERRORLEVEL -1073740771 ERRORLEVEL -1073740771
[00:33:41] at NUnitConf.checkTestResult(Result`2 result) in C:\projects\visualfsharp-3dtit\tests\fsharp\nunitConf.fs:line 14

Not sure why, it looks unrelated to your changes.

@wallymathieu

This comment has been minimized.

Show comment
Hide comment
@wallymathieu

wallymathieu Jul 16, 2016

Contributor

Quite odd indeed.

Contributor

wallymathieu commented Jul 16, 2016

Quite odd indeed.

@wallymathieu

This comment has been minimized.

Show comment
Hide comment
@wallymathieu

wallymathieu Jul 16, 2016

Contributor

That build looks a bit flaky.

Contributor

wallymathieu commented Jul 16, 2016

That build looks a bit flaky.

wallymathieu added some commits Jul 18, 2016

Merge branch 'RFC-FS-1004-Result-methods' of github.com:wallymathieu/…
…visualfsharp into RFC-FS-1004-Result-methods
@wallymathieu

This comment has been minimized.

Show comment
Hide comment
@wallymathieu

wallymathieu Jul 18, 2016

Contributor

Looks like all it needed was some random nudge. ;)

Contributor

wallymathieu commented Jul 18, 2016

Looks like all it needed was some random nudge. ;)

+ [<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
+ module Result =
+
+ /// <summary><c>map f inp</c> evaluates to <c>match inp with Error e -> Error e | Ok x -> Ok (f x)</c>.</summary>

This comment has been minimized.

@dsyme

dsyme Jul 22, 2016

Contributor

These summaries should be English-language. I know they're using the style from the "Option" summaries, but those ones should really be changed too :)

The MSDN docs use summary strings like Transforms an option value by using a specified mapping function.

@dsyme

dsyme Jul 22, 2016

Contributor

These summaries should be English-language. I know they're using the style from the "Option" summaries, but those ones should really be changed too :)

The MSDN docs use summary strings like Transforms an option value by using a specified mapping function.

@dsyme dsyme merged commit 047d472 into Microsoft:master Jul 22, 2016

4 checks passed

Windows_NT Debug Build Build finished.
Details
Windows_NT Release_ci_part1 Build Build finished.
Details
Windows_NT Release_ci_part2 Build Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jul 22, 2016

Contributor

Merging, will update the doc strings separately.

Contributor

dsyme commented Jul 22, 2016

Merging, will update the doc strings separately.

@cartermp

This comment has been minimized.

Show comment
Hide comment
@cartermp

cartermp Jul 22, 2016

Contributor

👍

Contributor

cartermp commented Jul 22, 2016

👍

@manofstick manofstick referenced this pull request Oct 14, 2016

Closed

[WIP] Seq Composer #1570

71 of 99 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment