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

Move refactoring contribution extensions to jdt.core.manipulation #914

Merged

Conversation

robstryker
Copy link
Contributor

… also small error checks

What it does

How to test

Author checklist

@rgrunber rgrunber added this to the 4.30 RC1 milestone Nov 9, 2023
@robstryker robstryker force-pushed the refactoringContributionsPluginXml branch 3 times, most recently from 82d4e54 to 62a6b38 Compare November 10, 2023 21:54
@robstryker robstryker marked this pull request as draft November 11, 2023 03:43
@robstryker robstryker force-pushed the refactoringContributionsPluginXml branch from 62a6b38 to d65cd4f Compare November 13, 2023 15:12
@robstryker
Copy link
Contributor Author

@jjohnstn This weekend, it was revealed that moving these extension point contributions down to jdt.core.manipulations led to many many strange and innaccurate API errors. Specifically, the api tools starts complaining about 100+ missing files (that are not missing at all, have not been deleted, and also have not even been moved at all.)

We may need to figure out how to run a build and skip the api tools for this specific patch? I don't think making a hundred problem filters for files that have not been deleted or moved is appropriate.

@iloveeclipse
Copy link
Member

it was revealed that moving these extension point contributions down to jdt.core.manipulations led to many many strange and innaccurate API errors. Specifically, the api tools starts complaining about 100+ missing files (that are not missing at all, have not been deleted, and also have not even been moved at all.)

@robstryker : as I wrote on #919 (comment), it's tycho problem, see eclipse-tycho/tycho#3019

There was a change in tycho that forces all bundles to define output paths in build.properties if they use API tooling, just see comments on linked issue.

@robstryker robstryker marked this pull request as ready for review November 13, 2023 17:40
Copy link
Contributor

@jjohnstn jjohnstn left a comment

Choose a reason for hiding this comment

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

Looks good.

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.

It's a +1 from me (assuming this is needed for the RC1 cycle)

Looking at https://github.com/eclipse-platform/eclipse.platform.ui/blob/master/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/internal/core/refactoring/history/RefactoringContributionManager.java#L164 , the contributions are still provided under the same namespace & id so there should be no noticeable difference. Also jdt.core.manipulation is guaranteed to be present where jdt.ui would be present so there's no way these would be missing.

@jjohnstn feel free to merge, though maybe a squash of the 2 commits beforehand.

@iloveeclipse
Copy link
Member

-1 from me to merge.
Such kind of changes is not for RC1.
It is not a critical bug and not a recent regression.
Please wait for 4.31 M1.

@jjohnstn
Copy link
Contributor

Just to clarify. I was assuming this was for 4.31 M1.

@rgrunber rgrunber removed this from the 4.30 RC1 milestone Nov 13, 2023
@jjohnstn jjohnstn added this to the 4.31 M1 milestone Nov 27, 2023
@rgrunber rgrunber force-pushed the refactoringContributionsPluginXml branch from 6476653 to 9ef5692 Compare November 28, 2023 17:40
@rgrunber rgrunber changed the title Move refactoring contribution extensions down into jdt.manipulations;… Move refactoring contribution extensions to jdt.core.manipulation Nov 28, 2023
- Also version bump jdt.core.manipulation

Signed-off-by: Rob Stryker <stryker@redhat.com>
@rgrunber rgrunber force-pushed the refactoringContributionsPluginXml branch from 9ef5692 to 92cb6fc Compare November 28, 2023 18:21
@rgrunber rgrunber merged commit 440dc9d into eclipse-jdt:master Nov 28, 2023
9 checks passed
@iloveeclipse
Copy link
Member

There is an API error caused by this PR:
The minor version should be the same for version 1.21.0, since no new APIs have been added since version 1.20.0

https://download.eclipse.org/eclipse/downloads/drops4/I20231128-1800/apitools/analysis/html/org.eclipse.jdt.core.manipulation/report.html

Since we can't undo published version (it was already in the I Build), please add API filter or propose a PR that actually changes API :-)

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.

4 participants