Rework default Server.fs - move to a module #465
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
theimowski
reviewed
Aug 23, 2021
| | Ok () -> return todo | ||
| | Error e -> return failwith e | ||
| } } | ||
| { |
There was a problem hiding this comment.
is this fantomas formatting?
| member __.GetTodos() = List.ofSeq todos | ||
|
|
||
| member __.AddTodo(todo: Todo) = | ||
| module Storage = |
There was a problem hiding this comment.
it looks more succinct and maybe even more idiomatic. I'm not convinced though - prefer to use explicit class/object notation when dealing with internal state. Probably a matter of preference though - curious what other think
There was a problem hiding this comment.
@theimowski in absence of any feedback (happy for you to loop in some people explicitly) - I would suggest we go with this one and add a recipe for the class-based (with DI) approach.
|
@theimowski I'm going to merge this in on Monday unless there's any real objections. |
|
Ok - feel free to do so 👍
sob., 13 lis 2021, 18:07 użytkownik Isaac Abraham ***@***.***>
napisał:
… @theimowski <https://github.com/theimowski> I'm going to merge this in on
Monday unless there's any real objections.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#465 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNCUFZDXFIO3W4PSR7GSKDUL2LLTANCNFSM5CMYVP5A>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
This was referenced Nov 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Small reformatting but also move from a class to a module and let-bound functions.