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.

Sign up for GitHub

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

Jump to bottom

Ready is Ready-Only, should only have getters. #90

Closed
Hoverbear opened this issue Jul 9, 2018 · 3 comments
Closed

Ready is Ready-Only, should only have getters. #90

Hoverbear opened this issue Jul 9, 2018 · 3 comments
Labels
Enhancement An improvement to existing code. Good First Issue A good issue for a new contributor. Help Wanted An issue with unsolved problems, looking for help.

Comments

@Hoverbear
Copy link
Contributor

The Ready struct should not allow values to be mutated, it should only provide immutable getters.

https://github.com/pingcap/raft-rs/blob/1e0741a9aea3b242d33dadc46245ad611e7e7aa4/src/raw_node.rs#L84-L121

It's possible to do this by writing functions that return immutable borrows of the contained values, or using a crate like https://crates.io/crates/getset which will write them for you.
@Hoverbear Hoverbear added Enhancement An improvement to existing code. Good First Issue A good issue for a new contributor. labels Jul 9, 2018
@Hoverbear Hoverbear mentioned this issue Jul 9, 2018
Merged
@Hoverbear Hoverbear added the Help Wanted An issue with unsolved problems, looking for help. label Sep 7, 2018
@Ryan-Git
Copy link
Contributor

Ryan-Git commented Sep 8, 2018

After adding getter, shall we remove pub modifier? It would be a breaking change then.

@siddontang
Copy link
Contributor

@Ryan-Git
Before 1.0 release, it is allowed to break things :-)

@Ryan-Git
Copy link
Contributor

Ryan-Git commented Sep 8, 2018

@siddontang
ok. I'll working on this.

@Ryan-Git Ryan-Git mentioned this issue Sep 9, 2018
Merged
bors bot added a commit that referenced this issue Nov 1, 2018
@bors @Ryan-Git
Merge #120
6fb68e4 
120: refactor(Ready): add pub getters and make fields private r=Hoverbear a=Ryan-Git

Fixes #90 

Co-authored-by: renhongdi <[email protected]>
Co-authored-by: Ryan-Git <[email protected]>
bors bot added a commit that referenced this issue Nov 2, 2018
@bors @Ryan-Git @Hoverbear
Merge #120
134b393 
120: refactor(Ready): add pub getters and make fields private r=Hoverbear a=Ryan-Git

Fixes #90 

Co-authored-by: renhongdi <[email protected]>
Co-authored-by: Ryan-Git <[email protected]>
Co-authored-by: A. Hobden <[email protected]>
bors bot added a commit that referenced this issue Nov 2, 2018
@bors @Ryan-Git @Hoverbear
Merge #120
d68ad38 
120: refactor(Ready): add pub getters and make fields private r=Hoverbear a=Ryan-Git

Fixes #90 

Co-authored-by: renhongdi <[email protected]>
Co-authored-by: Ryan-Git <[email protected]>
Co-authored-by: A. Hobden <[email protected]>
@bors bors bot closed this as completed in #120 Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement An improvement to existing code. Good First Issue A good issue for a new contributor. Help Wanted An issue with unsolved problems, looking for help.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Hoverbear @siddontang @Ryan-Git