-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Remove deprecated classes from app-model #41939
Conversation
Status for workflow
|
@gastaldi @aloubyansky just checking about RHBQ implications as the deprecation was done in 3.11 (thus not in the latest RHBQ 3.8) through #40224. Are these classes in scope of any support level from RHBQ side? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The methods that exposed the removed API seem to be deprecated a long time ago (2.4.0.CR1), so those could be removed.
We could still keep the AppArtifact*
classes around but they wouldn't be used in the quarkus repository at least. Theoretically, there could be uses of those in other extension repositories, which would be easy to fix.
If we want to be strict about this, we could add the classes back, we could also wait until somebody complains before doing that.
* @deprecated in favor of {@link #getKey()} | ||
* @return the artifact key or null if not available | ||
*/ | ||
AppArtifactKey getArtifactKey(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method was deprecated in 2.4.0.CR1
* @deprecated in favor of {@link #getKey()} | ||
* @return archive key | ||
*/ | ||
public AppArtifactKey getArtifactKey() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method was deprecated in 2.4.0.CR1
Sorry to be that guy but we need the deprecation to land in an LTS before a removal. |
These methods were deprecated since 2.x, we had 2 LTS with them being deprecated already, so I don't understand why we need to reintroduce them? |
My comment was about |
How about if we introduced an OpenRewrite script to migrate its usage? Anyway, not opposed to it, just considering options |
We can introduce an OpenRewrite recipe, that's indeed a good idea if feasible. But that doesn't change the rule :). |
That would be mostly for extension code though, not applications. |
The recipes also handle updating extensions. |
@gastaldi btw, I need the classes back for tomorrow. I don't want to release 3.13.0 without them. Thanks! |
I am not sure if it's too early to remove them but at least it would be good to have it gone in the next LTS