Skip to content

Refactor: Replace explicit type arguments with diamond operator wherever possible across the project#17567

Merged
kfaraz merged 8 commits intoapache:masterfrom
Akshat-Jain:use-simplified-diamond-operator-syntax
Dec 17, 2024
Merged

Refactor: Replace explicit type arguments with diamond operator wherever possible across the project#17567
kfaraz merged 8 commits intoapache:masterfrom
Akshat-Jain:use-simplified-diamond-operator-syntax

Conversation

@Akshat-Jain
Copy link
Copy Markdown
Contributor

@Akshat-Jain Akshat-Jain commented Dec 13, 2024

Description

As pointed out in this review comment, since we aren't supporting Java 8 anymore, we can switch to diamond operators wherever possible without specifying explicit type arguments.

Examples:

// Example 1:
private ArrayList<InputRow> longRows = new ArrayList<InputRow>();
// The above can be replaced with the following:
private ArrayList<InputRow> longRows = new ArrayList<>();

// Example 2:
new TypeReference<List<DataSegmentChangeRequest>>()
      {
      };
// The above can be replaced with the following:
new TypeReference<>() {};

This PR makes this change across the project.

Note: This change was done with IntelliJ's project-wide inspection results for warnings like Explicit type argument Integer, Iterator<Integer> can be replaced with <> . With that, there were a few compilation issues, which were manually fixed.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

Copy link
Copy Markdown
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked at some of the files, several more to go.
We should probably leave out the formatting changes to simplify the PR.

@Akshat-Jain
Copy link
Copy Markdown
Contributor Author

Looked at some of the files, several more to go. We should probably leave out the formatting changes to simplify the PR.

@kfaraz Have reverted all formatting changes across all files.

@Akshat-Jain Akshat-Jain requested a review from kfaraz December 16, 2024 07:50
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all files.
LGTM! 🚀

@kfaraz
Copy link
Copy Markdown
Contributor

kfaraz commented Dec 17, 2024

Merging since the failures are unrelated.

@kfaraz kfaraz merged commit 98b960c into apache:master Dec 17, 2024
@kgyrtkirk
Copy link
Copy Markdown
Member

@Akshat-Jain
Copy link
Copy Markdown
Contributor Author

@kgyrtkirk Makes sense, thanks for pointing this out! Will do this in a follow-up!

@adarshsanjeev adarshsanjeev added this to the 32.0.0 milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants