Skip to content

Add MutableState Protocol#190

Merged
tinder-garricnahapetian merged 32 commits intomainfrom
add-mutablestate-protocol
Mar 31, 2023
Merged

Add MutableState Protocol#190
tinder-garricnahapetian merged 32 commits intomainfrom
add-mutablestate-protocol

Conversation

@seppesnoeck
Copy link
Contributor

This PR will add a new MutableState Protocol.

@seppesnoeck seppesnoeck self-assigned this Oct 21, 2022
Copy link
Contributor

@tinder-cfuller tinder-cfuller left a comment

Choose a reason for hiding this comment

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

May you please add DocC comments to the methods in both the protocol and the extension? And please add two unit tests as well. Thank you!

Copy link
Contributor

@tinder-garricnahapetian tinder-garricnahapetian left a comment

Choose a reason for hiding this comment

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

Please consider adding documentation.

Copy link
Contributor

@tinder-garricnahapetian tinder-garricnahapetian left a comment

Choose a reason for hiding this comment

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

Please consider adding tests.

@tinder-garricnahapetian tinder-garricnahapetian marked this pull request as ready for review March 27, 2023 22:24
@tinder-garricnahapetian tinder-garricnahapetian requested review from tinder-cfuller and removed request for tinder-cfuller March 27, 2023 23:10
tinder-cfuller
tinder-cfuller previously approved these changes Mar 30, 2023
Copy link
Contributor

@tinder-cfuller tinder-cfuller left a comment

Choose a reason for hiding this comment

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

Great! ⭐

Copy link
Contributor

@tinder-cfuller tinder-cfuller left a comment

Choose a reason for hiding this comment

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

@tinder-garricnahapetian One last thought, I believe we should consider the name applying instead of with for the non-mutating method. This would follow Apple's naming guidelines I believe.

tinder-garricnahapetian and others added 2 commits March 30, 2023 15:01
Co-authored-by: Christopher Fuller <christopher.fuller@gotinder.com>
Co-authored-by: Christopher Fuller <christopher.fuller@gotinder.com>
Co-authored-by: Christopher Fuller <christopher.fuller@gotinder.com>
Co-authored-by: Christopher Fuller <christopher.fuller@gotinder.com>
Copy link
Contributor

@tinder-cfuller tinder-cfuller left a comment

Choose a reason for hiding this comment

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

🚀

@tinder-garricnahapetian tinder-garricnahapetian merged commit 0f9fb7d into main Mar 31, 2023
@tinder-garricnahapetian tinder-garricnahapetian deleted the add-mutablestate-protocol branch March 31, 2023 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

framework Framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants