-
Notifications
You must be signed in to change notification settings - Fork 413
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
Conversation
3b97fae
to
0d9f1a6
Compare
@@ -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)); |
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'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
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 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.
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'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
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.
@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.
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'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>
0d9f1a6
to
b33f3cc
Compare
No description provided.