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

Removed Internal Mutability #42

Merged

Conversation

jonahd-g
Copy link
Contributor

@jonahd-g jonahd-g commented Jan 17, 2021

Removed requirement on internal mutability by using mutable self references in methods.

See #39 for details. Should not be merged unless that alternate approach to sharing is approved.

/resolves #39

@ryan-summers
Copy link
Member

I agree with your point about internal mutability being required and the existing API not guaranteeing thread safety, and thus, I am on board with this. @MathiasKoch thoughts?

@MathiasKoch
Copy link
Collaborator

I think this is okay, assuming we can reach some agreement on #39 as stated in OP

This was referenced Jan 18, 2021
Copy link
Member

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

Ah just noticed missing CHANGELOG entry. Mind adding a notice there that we're adding self mutability and reference issue #39 for more detail?

Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

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

I agree with this way forward.
Other than the changelog entry, looks good to me. Thanks!

Copy link
Member

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

Thanks! :)

@jonahd-g
Copy link
Contributor Author

Thanks Ryan. I keep forgetting too, maybe we need an Action to check for that.

Changelog entry added.

@ryan-summers ryan-summers merged commit 0bc4d77 into rust-embedded-community:master Jan 19, 2021
@jonahd-g jonahd-g deleted the no-internal-mutability branch January 19, 2021 18:04
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.

Revisit on mutable self references
5 participants