JakeWharton
commented
12 months ago Hello. Thanks for attempting to improve our project, but this type of PR is not acceptable let alone reviewable as-is.
I'm not sure if this is part of an assignment, you are demonstrating a tool, or a combination of both. But immediately there are giant red flags from a cursory look at the diff:
- Checked in generated files from a tool that we do not use
- Changed public API of the library in an incompatible way
- Used different indentation and general code style as the rest of the library
There may be individual changes within this diff that are valid, but they get drowned in the overwhelming size of the diff. This is not a good way to approach an open source project, especially one that is a decade old and well established. Changes proposed to any project should always be as small as possible, have clear motivations, and ideally are tested. Here is an example which was recently merged: https://github.com/square/javapoet/pull/986. Using these code quality tools and applying their advice blindly is not a good way to have your changes integrated. Moreover, these tools are rarely pragmatic and instead flag plenty of things which are perfectly fine in a real codebase.
If you are following the instruction of a professor in the creation of this PR, please forward my feedback to them. They are doing you a great disservice. There is plenty of opportunity to contribute to open source as an education experience, but this is not the way.
Refactored classes and removed implementation and design smells.