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

Report whether the project has Kotlin Gradle files. #2859

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

rgrunber
Copy link
Contributor

No description provided.

@@ -152,6 +163,9 @@ public void onBuildFinished(long buildFinishedTime) {
buildFinishedElapsedTime = buildFinishedTime - this.languageServerStartTime;

properties.add("buildToolNames", buildToolNamesList);
if (hasKotlinGradleFiles) {
properties.addProperty("hasKotlinGradleFiles", Boolean.toString(hasKotlinGradleFiles));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not fond of build tool specific code. I'd rather prefer we stay build tool agnostic as much as possible.
I'd rather prefer something like buildFile=pom.xml|build.gradle|build.gradle.kts|... if possible

Copy link
Contributor Author

@rgrunber rgrunber Sep 21, 2023

Choose a reason for hiding this comment

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

It could be done, but I'm not sure what relevant data we'd get for non-Gradle projects.

- All Maven projects would be "pom.xml" at the top-level. I could report it just for the sake of doing so
- All Eclipse projects could be "build.properties", or just "none". Is it worth reporting whether an Eclipse project has a top-level pom file even though the MavenProjectImporter (runs with priority over EclipseProjectImporter) failed to detect it as a Maven project ?
- For invisible projects there's "none"

Gradle is the only really interesting options as it permits 2 different styles of build. Or would you rather I report any build file present at the root of a project regardless of what the chosen build tools happens to be ?

Ok, I'll just report any build files in a given project. It could be interesting to be aware of cases where we've chosen a given build tool, but other options might have worked. Currently we try Gradle -> Maven -> Invisible -> Eclipse.

Copy link
Contributor

@fbricon fbricon Sep 21, 2023

Choose a reason for hiding this comment

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

I'm not convinced that would be actionable/useful.
Let's keep the buildFile info for Gradle only for now.
You could add a new default method to the IBuildSupport that reports build files found, or introduce a new interface only the Gradle support would implement

Copy link
Contributor Author

@rgrunber rgrunber Sep 21, 2023

Choose a reason for hiding this comment

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

@fbricon Ok, this should be fixed now. buildFileNames only reports for Gradle projects and we can report a list in case we need to expand to multiple types of build files in the future.

Copy link
Contributor Author

@rgrunber rgrunber Sep 26, 2023

Choose a reason for hiding this comment

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

I'll merge this for now, and in the future if we really want to know more about other build system files, we could add to IBuildSupport. Frameworks as opposed to just files would also be interesting to detect.

- Report whether "build.gradle" or "build.gradle.kts" is used within the
  project

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber rgrunber merged commit a2537b4 into eclipse-jdtls:master Sep 26, 2023
6 checks passed
@rgrunber rgrunber deleted the detect-gradle-kotlin branch September 26, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants