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

Privatize temporarily public (formerly implicit) class properties #748

Closed
iansltx opened this issue Oct 25, 2019 · 0 comments · Fixed by #750
Closed

Privatize temporarily public (formerly implicit) class properties #748

iansltx opened this issue Oct 25, 2019 · 0 comments · Fixed by #750

Comments

@iansltx
Copy link
Contributor

iansltx commented Oct 25, 2019

Resolution for this issue would include code that effectively reverts #747, switching class properties to private (or, if inherited and it makes sense to leave them that way, protected) that used to be implicity set (and thus public). In order for this to happen, each class instance will need to be audited to ensure that all public property accesses from outside that class go through getters, and all changes (if any) go through setters. Bonus points if this results in more test coverage and/or type declarations in affected code.

iansltx added a commit to iansltx/joindin-api that referenced this issue Oct 26, 2019
Resolves joindin#748. Confirmed that child controllers use getters on both of
these mappers so they're save to make private rather than protected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant