-
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
Include relevant upstream catalogs even if their versions aren't recommended for new projects #19908
Include relevant upstream catalogs even if their versions aren't recommended for new projects #19908
Conversation
@gastaldi do you mind reviewing this one? Thanks! |
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.
LGTM, added a minor comment
* @deprecated in favor of {@link io.quarkus.registry.catalog.PlatformStreamCoords} | ||
*/ | ||
@Deprecated | ||
public class StreamCoords extends io.quarkus.registry.catalog.PlatformStreamCoords { |
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.
I think this can be removed completely? It's not being used anywhere AFAIK
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.
You are most probably right. I kept it, just in case, for backward compatibility.
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.
Right, my point is that deleting it shouldn´t break backward compatibility since it's already well encapsulated from the public APIs. It's unlikely that someone is using it since it was introduced less than 3 months ago 😉
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.
It shouldn't affect our dev tools, so yes, I think it's safe to remove it.
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.
Removed it.
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.
@ia3andy just to confirm, this class is not used in code.quarkus.io, right?
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.
It doesn't look like it.
9df8576
to
dd1081c
Compare
…mmended for new projects Moved io.quarkus.maven.StreamCoords to io.quarkus.registry.catalog.PlatformStreamCoords
dd1081c
to
9a15b82
Compare
The errors aren't related to the change. E.g.
|
It may happen that downstream platform versions reference upstream Quarkus versions that are not recommended any more for new projects in the upstream community. This PR fixes a few issues making sure the dev tools still list the relevant upstream catalogs for downstream versions.
Also deprecated io.quarkus.maven.StreamCoords in favor of io.quarkus.registry.catalog.PlatformStreamCoords