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

Remove public modifier from groovy methods #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dustinkredmond
Copy link

Groovy methods don't need the public modifier, as they're public by default. This commit removes the public from each method where it is not needed.

@Dierk
Copy link
Member

Dierk commented Oct 2, 2020

looks good to me. It is important not to remove the public keyword from e.g. field/property declarations but that seems to be observed.
The change makes the code more idiomatic, yet less explicit. I'm undecided whether that is an improvement.

@aalmiray
Copy link
Member

aalmiray commented Oct 2, 2020

I'm undecided as well. I hope this is not part of the Hacktoberfest event, in which case and if the contribution has merit then it'll be merged after Hacktoberfest has ended.

@dustinkredmond
Copy link
Author

@Dierk I agree that it does make it less explicit, although seeing public throughout 95% of the Groovy classes does make it far less idiomatic, and less (no pun intended) Groovy. I'm seeing more and more projects, especially new ones, that have adopted dropping the unnecessary public modifier, and was hoping that groovyfx will be able to contend with these going forward, as it looks like JavaFX is here to stay.

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.

3 participants