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

Incremental delete of duplicate interfaces (Delete ICleanUpCore) #1161

Conversation

robstryker
Copy link
Contributor

@robstryker robstryker commented Feb 1, 2024

What it does

Removes internal interface ICleanupCore as a duplicate.

This PR depends on #1138 as it touches the same code

Why this is needed

While pushing code related to the completion subprocessors from jdt.ui to jdt.core.manipulations, one specific (and hefty) subprocessor, LocalCorrectionsSubProcessor, uses a lot of proposals that wrap CleanUp objects, using either the ICleanUp or ICleanUpCore interfaces, as well as the AbstractCleanUp or AbstractCleanUpCore abstract classes.

While identical interfaces are not much of a problem, having two parallel-but-slightly-different abstract classes leads to two parallel heirarchies, and clients can not simply extend both or support both very easily. While trying to untangle these specific issues, the myriad of other duplicate (and otherwise harmless) interfaces that are in the affected code also begin to add complexity to the solution.

With that in mind, removing the most unoffensive interfaces first becomes the safest course of action, while at the same time building our way up to tackling the larger parallel-heirarchy issue via small and manageable changes to the heirarchy as the situation becomes less cluttered.

How to test

Author checklist

@robstryker robstryker changed the title Incremental delete of duplicate interfaces part2 Incremental delete of duplicate interfaces part2 (Delete ICleanUpCore) Feb 1, 2024
@robstryker robstryker force-pushed the IncrementalDeleteOfDuplicateInterfacesPart2 branch 2 times, most recently from 47d29c4 to cef5bea Compare February 2, 2024 16:33
Rob Stryker added 2 commits February 7, 2024 16:23
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
@robstryker robstryker force-pushed the IncrementalDeleteOfDuplicateInterfacesPart2 branch 2 times, most recently from d0010ca to fe7f54f Compare February 8, 2024 15:08
Signed-off-by: Rob Stryker <stryker@redhat.com>

Missed a file

Signed-off-by: Rob Stryker <stryker@redhat.com>
@robstryker robstryker force-pushed the IncrementalDeleteOfDuplicateInterfacesPart2 branch from fe7f54f to 1f28f80 Compare February 8, 2024 16:10
…does

Signed-off-by: Rob Stryker <stryker@redhat.com>
@rgrunber rgrunber added this to the 4.31 M3 milestone Feb 8, 2024
@rgrunber rgrunber self-requested a review February 8, 2024 20:14
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

I've tested this out by playing with cleanup actions (and cleanup on save), and it all seems to be working as expected. That combined with the various cleanup tests makes me inclined to merge.

Signed-off-by: Rob Stryker <stryker@redhat.com>
@rgrunber rgrunber merged commit ba75582 into eclipse-jdt:master Feb 9, 2024
7 checks passed
@rgrunber rgrunber changed the title Incremental delete of duplicate interfaces part2 (Delete ICleanUpCore) Incremental delete of duplicate interfaces (Delete ICleanUpCore) Feb 9, 2024
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

Successfully merging this pull request may close these issues.

2 participants