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

SubModelSeq update logic now more functional #241

Merged
merged 1 commit into from
Jul 25, 2020

Conversation

TysonMN
Copy link
Member

@TysonMN TysonMN commented Jul 22, 2020

This PR only changes code in the SubModelSeq sample.

At a high level, the changes are to use more higher-order functions in order to be both more succinct and have more reusable code.

My intention is for the behavior to be unchanged. I also expect the performance to be the same. These changes are not trying to address #236.

I tested this, and I think everything still works the same.

@ScottHutchinson, I think you will be interested in these changes since you are using a SubModelSeq binding.

@TysonMN TysonMN force-pushed the improve_SubModelSeq_update branch from 3ff5994 to 5fdbdc9 Compare July 22, 2020 12:18
@TysonMN TysonMN force-pushed the improve_SubModelSeq_update branch from 5fdbdc9 to 1ea4d1f Compare July 22, 2020 17:18
Copy link
Member

@cmeeren cmeeren left a comment

Choose a reason for hiding this comment

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

Wasn't able to look at everything, will continue another time. Feel free to work on the current suggestions, though.

@TysonMN TysonMN force-pushed the improve_SubModelSeq_update branch 2 times, most recently from 47274b0 to 0c05717 Compare July 23, 2020 01:15
@TysonMN TysonMN force-pushed the improve_SubModelSeq_update branch 2 times, most recently from 04c74fb to b1bc3a2 Compare July 23, 2020 17:03
Copy link
Member

@cmeeren cmeeren left a comment

Choose a reason for hiding this comment

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

More or less good to go. Just a few things left.

@cmeeren
Copy link
Member

cmeeren commented Jul 25, 2020

I'm happy with this. Feel free to merge. Thanks for your work!

@TysonMN TysonMN force-pushed the improve_SubModelSeq_update branch from f2e76a8 to 41e2207 Compare July 25, 2020 18:38
@TysonMN
Copy link
Member Author

TysonMN commented Jul 25, 2020

I locally squashed and rebased onto master. I will complete the PR after the build succeeds.

@TysonMN TysonMN merged commit 76c96c2 into elmish:master Jul 25, 2020
@TysonMN TysonMN deleted the improve_SubModelSeq_update branch July 25, 2020 18:46
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.

2 participants