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.

Sign up for GitHub

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

Jump to bottom

ArC: remove deprecated classes and methods #33907

Merged
merged 11 commits into from
Jun 9, 2023
Merged

ArC: remove deprecated classes and methods #33907

merged 11 commits into from
Jun 9, 2023

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Jun 8, 2023

No description provided.

@mkouba mkouba added the area/housekeeping Issue type for generalized tasks not related to bugs or enhancements label Jun 8, 2023
@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/core labels Jun 8, 2023
@Ladicek
Copy link
Contributor

Ladicek commented Jun 8, 2023

ProxyUnwrapperBuildItem was deprecated in 3.0, so this is fairly soon IMHO. My personal opinion is that 6 months/releases is a reasonable period of time (which would be exactly the case for ClientProxyUnwrapper that got deprecated in 2.13). WDYT?

That's my only reservation about this, so if you think it's fine, I of course won't block it :-)

@mkouba
Copy link
Contributor Author

mkouba commented Jun 8, 2023

ProxyUnwrapperBuildItem was deprecated in 3.0, so this is fairly soon IMHO. My personal opinion is that 6 months/releases is a reasonable period of time (which would be exactly the case for ClientProxyUnwrapper that got deprecated in 2.13). WDYT?

That's my only reservation about this, so if you think it's fine, I of course won't block it :-)

I do want to remove as much deprecated stuff as possible for the 3.2 (aka LTS).

@mkouba
Copy link
Contributor Author

mkouba commented Jun 8, 2023

io.quarkus.arc.Priority is another good candidate...

@manovotn
Copy link
Contributor

manovotn commented Jun 8, 2023

I do want to remove as much deprecated stuff as possible for the 3.2 (aka LTS).

I am fully behind this 👍
We did the deprecations for a reason and an LTS is a good cut-off point at which we should perform a removal so that we don't carry on with the legacy for far too long.

@mkouba mkouba changed the title Remove the deprecated ProxyUnwrapperBuildItem and related classes ArC: remove deprecated classes and methods Jun 8, 2023
@quarkus-bot

This comment has been minimized.

Sign in to view
@mkouba mkouba force-pushed the arc-deprecated-proxyunwrapper branch from 24e0e4f to d122311 Compare June 8, 2023 13:59
@github-actions
Copy link

github-actions bot commented Jun 8, 2023 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@mkouba mkouba requested review from Ladicek and manovotn June 8, 2023 15:47
@quarkus-bot

This comment has been minimized.

Sign in to view
manovotn
manovotn reviewed Jun 9, 2023
View reviewed changes
Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

Just a few minor comments WRT to now superfluous tests

...c/test/java/io/quarkus/arc/test/alternatives/priority/AlternativePriorityResolutionTest.java Show resolved Hide resolved
independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/instance/PriorityTest.java Show resolved Hide resolved
...ests/src/test/java/io/quarkus/arc/test/stereotypes/StereotypeAlternativeArcPriorityTest.java Outdated Show resolved Hide resolved
@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 9, 2023
Ladicek
Ladicek approved these changes Jun 9, 2023
View reviewed changes
Copy link
Contributor

@Ladicek Ladicek left a comment

Choose a reason for hiding this comment

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

LGTM (barring the comment about perhaps removing some deprecations too quickly)

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@manovotn
Copy link
Contributor

manovotn commented Jun 9, 2023

Failed to execute goal net.revelc.code:impsort-maven-plugin:1.9.0:check (check-imports) on project quarkus-arc-deployment: Imports are not sorted in /home/runner/work/quarkus/quarkus/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java

I am unable to reproduce this locally, nor do I get any additional imports sorted during the build 🤷

@mkouba mkouba force-pushed the arc-deprecated-proxyunwrapper branch from 20e8086 to 22620b6 Compare June 9, 2023 10:37
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 9, 2023

Failing Jobs - Building 22620b6

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
✖ JVM Tests - JDK 17 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 19
⌛ Native Tests - HTTP Build ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 Windows #

- Failing: extensions/amazon-lambda/deployment 
! Skipped: extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment integration-tests/amazon-lambda and 6 more

📦 extensions/amazon-lambda/deployment

✖ io.quarkus.amazon.lambda.deployment.testing.LambdaDevServicesContinuousTestingTestCase.testLambda line 48 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <1> but was: <0>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)

@mkouba mkouba merged commit a7dc8ea into quarkusio:main Jun 9, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 9, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manovotn manovotn manovotn approved these changes

@Ladicek Ladicek Ladicek approved these changes

Assignees
No one assigned
Labels
area/arc Issue related to ARC (dependency injection) area/core area/documentation area/housekeeping Issue type for generalized tasks not related to bugs or enhancements
Projects
None yet
Milestone
3.2.0.CR1
Development

Successfully merging this pull request may close these issues.
3 participants
@mkouba @Ladicek @manovotn