Skip to content

Remove intellij-inspections check from CI#17469

Merged
cryptoe merged 5 commits intoapache:masterfrom
Akshat-Jain:remove-intellij-inspections-check
Nov 13, 2024
Merged

Remove intellij-inspections check from CI#17469
cryptoe merged 5 commits intoapache:masterfrom
Akshat-Jain:remove-intellij-inspections-check

Conversation

@Akshat-Jain
Copy link
Copy Markdown
Contributor

Description

We plan to deprecate Java 8 as part of #17466. We ran into some issues with intellij-inspections check while doing that.

On further discussion, we decided that we should remove intellij-inspections check for the following reasons:

  1. intellij-inspections is coming from https://github.com/ccaominh/intellij-inspect, which is tied down to openjdk8 image.
  2. The image for intellij-inspect is published at https://hub.docker.com/r/ccaominh/intellij-inspect, which is outside of Apache infra.
  3. The above project isn't being actively maintained.
  4. We also have other checks (like maven-checkstyle-plugin and spotbugs) integrated in our CI pipeline which makes the intellij-inspections check redundant for the most part.

On further looking into point (4), the intellij-inspections check does seem to provide some useful checks which aren't being covered in other static checks we have at the moment.

So the plan is:

  1. Remove intellij-inspections check.
  2. Remove Java 8 support.
  3. Explore other options like Qodana and integrate them in the repo as a replacement for intellij-inspections check.

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.

@github-actions github-actions Bot added the GHA label Nov 13, 2024
@cryptoe
Copy link
Copy Markdown
Contributor

cryptoe commented Nov 13, 2024

Why do we need to remove these files

Folks should be able to run intellij inspections locally if they want to.

Copy link
Copy Markdown
Contributor

@cryptoe cryptoe left a comment

Choose a reason for hiding this comment

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

Lets not remove the .idea files.

@cryptoe cryptoe merged commit 390c2d6 into apache:master Nov 13, 2024
@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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants