ExtCore icon indicating copy to clipboard operation
ExtCore copied to clipboard

Both Result and Choice for error handling

Open wallymathieu opened this issue 8 years ago • 15 comments

  • Forked individual files from #31 and reverted original files
  • Added Compatibility namespaces and moved some Choice constructs into these
  • Renamed Notification.Error to Notification.Exception in order to avoid confusing things with Result<,>

wallymathieu avatar Oct 30 '17 19:10 wallymathieu

Note that I've added same kind of test as can be found for Fold for FoldBack @jack-pappas

wallymathieu avatar Oct 30 '17 19:10 wallymathieu

@vasily-kirichenko and @jack-pappas thoughts? Does it look ok?

wallymathieu avatar Oct 30 '17 20:10 wallymathieu

Thanks @wallymathieu — I’ll take a look at this over the weekend.

jack-pappas avatar Nov 02 '17 12:11 jack-pappas

@jack-pappas could you please publish a new NuGet package after merging this PR?

vasily-kirichenko avatar Nov 02 '17 12:11 vasily-kirichenko

https://gist.github.com/wallymathieu/ad8fe3d5b85875cf6f487ad0180be660 ?

wallymathieu avatar Nov 04 '17 22:11 wallymathieu

Should we perhaps try to split this pull request into more granular parts? A lot changes are mostly copy and modify (in order to be compatible). @vasily-kirichenko do you have an idea of how this could be done?

wallymathieu avatar Nov 11 '17 17:11 wallymathieu

Is there a plan to merge this?

vasily-kirichenko avatar Dec 25 '17 07:12 vasily-kirichenko

Should we close this pull request @jack-pappas (as it is to big) ?

wallymathieu avatar Jan 27 '18 07:01 wallymathieu

I'm closing this PR as it is probably to big to review or accept. Best of luck!

wallymathieu avatar Mar 19 '18 15:03 wallymathieu

@jack-pappas Please, merge this PR. I had to build an ExtCore package myself and publish it on a private server, everything's been working OK.

vasily-kirichenko avatar Jul 15 '18 17:07 vasily-kirichenko

Since @vasily-kirichenko says that it's working fine, I've reopened the pull request

wallymathieu avatar Nov 02 '18 10:11 wallymathieu

Yes, it's working 100% fine.

vasily-kirichenko avatar Nov 02 '18 10:11 vasily-kirichenko

Then perhaps move this PR to point to another branch on this repository in order to iterate on it?

wallymathieu avatar Nov 09 '18 07:11 wallymathieu

@wallymathieu Could you merge master into it to make AppVeyor happy?

vasily-kirichenko avatar Nov 09 '18 07:11 vasily-kirichenko

Will do!

wallymathieu avatar Nov 09 '18 07:11 wallymathieu