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

Would you like "Style" PRs that make some classes final? #6

Open
rbygrave opened this issue Dec 3, 2024 · 4 comments
Open

Would you like "Style" PRs that make some classes final? #6

rbygrave opened this issue Dec 3, 2024 · 4 comments

Comments

@rbygrave
Copy link

rbygrave commented Dec 3, 2024

Hi, there are some classes that look to me like they could be final. Are you interested in getting PR's for adding the final modifier to some of these classes?

As another style note, I see some methods that implement an interface method are not annotated with @Override. Are you interested in PR's that would add @Override to these methods [conceptually this improves support for refactoring such as removing methods from interfaces].

Are either of these something you'd like to get PR's for?

Cheers, Rob.

@robaho
Copy link
Owner

robaho commented Dec 3, 2024

Honestly, it depends… a lot of the structure, classnames, etc are taken directly from the JDK code. I only deviated when it was absolutely necessary in order to limit the scope of changes the JDK team would need to review if they wanted incorporate.

You are more than welcome to do a preview PR so I can review more specifics of where/what you want to fix.

@rbygrave
Copy link
Author

rbygrave commented Dec 3, 2024

Here is a preview PR / example - #7

@robaho
Copy link
Owner

robaho commented Dec 5, 2024

I took a look. I’m ok with PRs like this as long as they do not involve the original source classes from the JDK. For instance, the ChunkedOutputStream comes from there.

@rbygrave
Copy link
Author

rbygrave commented Dec 6, 2024

Ok cool. I'll update that PR and see how we go.

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

No branches or pull requests

2 participants