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

Do not rely on Quarkus BOM to resolve the version for analytics #45477

Closed
wants to merge 1 commit into from
Closed

Do not rely on Quarkus BOM to resolve the version for analytics #45477

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...-projects/tools/analytics-common/src/main/java/io/quarkus/analytics/AnalyticsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ private boolean allEnvSet(String... names) {
}

private String getQuarkusVersion(ApplicationModel applicationModel) {
return applicationModel.getPlatforms().getImportedPlatformBoms().stream()
.filter(artifactCoords -> artifactCoords.getArtifactId().equals("quarkus-bom"))
return applicationModel.getDependencies().stream()
.filter(artifactCoords -> artifactCoords.getArtifactId().equals("quarkus-bootstrap-core"))
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think @aloubyansky ?

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't do this, assuming we want the platform version. But I'm curious why the version is not detectable. It also depends what we want as "Quarkus version".

The difference is that, currently it's the platform version and while this will be the core, that may not be aligned downstream, just in case.

Second point, it's not exactly about direct quarkus-bom imports. Even there is a hierarchy of parent POMs and BOM imports, including flattening, the quarkus-bom should be detectable unless some (fingerprinting) part of it is removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

right, we want platform version

Copy link
Member Author

@rsvoboda rsvoboda Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

There is noticeable portion of N/A versions reported in build time analytics, so I thought it's because of custom bom. We should look into the reasons why https://github.com/quarkusio/quarkus/blob/main/independent-projects/tools/analytics-common/src/main/java/io/quarkus/analytics/AnalyticsService.java#L260 resolves the version as N/A.

My theory was false :/ I tried my custom BOM and io.quarkus.platform:quarkus-bom was reported by applicationModel.getPlatforms().getImportedPlatformBoms()

Can it be that people use custom builds of Quarkus?

Copy link
Member

Choose a reason for hiding this comment

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

Can it be that people use custom builds of Quarkus?

Theoretically. But given the amount, I doubt that's the reality.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree there are too many.

.map(ArtifactCoords::getVersion)
.findFirst()
.orElse("N/A");
Expand Down
Loading