Minor code cleanup (unused imports & static/final order) — OK to submit PR?

Elastic Stack Elasticsearch
1

Dear developers,

I found a few places in the code where there are unused imports and static final modifiers out of the recommended order.
I'd like to open a PR to clean these up to align with the code style and remove any unnecessary imports.
However, I'm aware that sometimes minor refactoring or purely stylistic changes can be considered too trivial.

  • Is it okay if I open a PR for these changes?
  • Would you prefer I include these changes in a larger PR or separate them out?

Thanks for your guidance!

2

Hi @LTS2 and welcome to the community!

You're right, sorry, we generally reject such PRs, they're not totally trivial to review and it's not really worth the effort.

However we do enforce code style in the build, including rejecting code with unused imports and disordered modifiers, so I'm kinda surprised to hear that you have found some instances of these style errors. Would you share a diff here (or link to a branch in your fork in Github) so we can see what you mean? It could be that you've found a bug in the style check, and that would be worth reporting if so.

Would you prefer I include these changes in a larger PR or separate them out?

If you have a larger change to propose then ideally we'd want to see a PR that just does one thing and doesn't include other unrelated fixes. It's much easier to review a PR that only addresses one thing at once.

3

Thank you for your feedback. Based on your recommendation, I've created a small, standalone PR that only includes the changes for removing unused imports and reordering the static final modifiers. You can review the diff here:

I appreciate your guidance and look forward to your feedback.

4

I see, thanks, that helps. Yes we wouldn't accept a PR that just does this cleanup. But this also does indicate a bug in the style check config: ./gradlew spotlessApply should make these changes automatically but it seems it's not doing so. A PR to fix the spotless config would be acceptable I think.

1 Like