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

Add F# examples #92

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

purkhusid
Copy link

What was changed

Added a F# example that mirrors the ActivitySimple C# sample

Why?

So that people that use F# can more easily see how Temporal code looks

@CLAassistant
Copy link

CLAassistant commented Jan 10, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cretz
Copy link
Member

cretz commented Jan 10, 2025

Thanks, this is great! I had some trouble with lambda expressions and F# in the past, happy to see this is working. I assume you manually confirmed it worked? Three requests:

  • Update the primary README.md pointing to this directory as we do with the other samples
  • Update .editorconfig to ignore CA1515 (or this PR can wait on Update .NET SDK and add update-with-start samples #91 to be merged where we did this at 23eecf7)
  • Is it possible to write a test? Can follow what we do in tests/ActivitySimple? Ideally F# can be in the same project, but if not, maybe we need a new tests-fsharp project? If this is too much effort we can do in a separate issue.

@purkhusid
Copy link
Author

Thanks, this is great! I had some trouble with lambda expressions and F# in the past, happy to see this is working. I assume you manually confirmed it worked? Three requests:

* Update the primary `README.md` pointing to this directory as we do with the other samples

* Update `.editorconfig` to ignore CA1515 (or this PR can wait on [Update .NET SDK and add update-with-start samples #91](https://github.com/temporalio/samples-dotnet/pull/91) to be merged where we did this at [23eecf7](https://github.com/temporalio/samples-dotnet/commit/23eecf7f4dbb494cb92c6f19c8202d7a7f996f16))

* Is it possible to write a test? Can follow what we do in `tests/ActivitySimple`? Ideally F# can be in the same project, but if not, maybe we need a new `tests-fsharp` project? If this is too much effort we can do in a separate issue.

Thanks for taking a look. This works but I'm looking into adding a F# example for some of the more complicated examples as well. The overload resolution is quite painful in some cases so I might create a supporting library with F# utilities if I discover some nice patterns.

I'll also add some F# tests as well. I'll let you know once it's ready for review again.

@cretz
Copy link
Member

cretz commented Jan 14, 2025

The overload resolution is quite painful in some cases so I might create a supporting library with F# utilities if I discover some nice patterns.

I hit this too a while back (a couple of years ago in a research branch). While I am not sure we're yet at a place where we want to maintain a Temporalio.Extensions.FSharp library in the primary SDK, putting generic extensions/utilities in a class in the sample here can help others. But don't feel obligated to do it here/now. And if we need to tweak the SDK in ways to make them work, we can discuss it.

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.

3 participants