-
Notifications
You must be signed in to change notification settings - Fork 54
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
[1.8.x]Update README.md and Ballerina.toml files #1550
Conversation
[platform.java17] | ||
graalvmCompatible = true |
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.
How did we verify that the module is graalVM compatible? Have we configured a graalVM workflow to run module test using the native executable?
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.
Since we only have ballerina code here, we can confirm its compatible 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.
My mistake. We do have java dependencies in the module.
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.
yeah we do use our core modules in here. so might need to come up with atleast a basic test suite to run on graalVM before marking as graalVM-compatible
.
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 curious about adding tests to verify GraalVM compatibility, especially since we have CLI and OpenAPI validator functionality that operates during compilation. I'll look into incorporating tests to ensure compatibility with GraalVM.
Appreciate it if anyone has any references or insights related to GraalVM compatibility testing, please feel free to share them :)
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.
Looping in @TharmiganK
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 is tool right? Since we are not executing the bal tools as GraalVM native images it is not required to be GraalVM compatible
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.
@TharmiganK this is not the tool. We do have this ballerina/openapi
library module to provide some annotation related to our openAPI validation support. So I think we should consider marking it as graalvm compatible (or not).
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 see
Are these annotations only relevant when we call OAS generation?
If we do not have any runtime use cases we can mark it as GraalVM compatible to avoid the warnings when building (bal build/test --native
or bal pack
)
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.
Btw why do we have a java dependency in the Ballerina.toml
? I dont see any external calls to java
[[platform.java17.dependency]] |
Isn't this for the compiler plugin? I saw the same jar in the CompilerPlugin.toml
:
path = "../openapi-validator/build/libs/[email protected]@.jar" |
I think just removing it from the Ballerina.toml
should work
|
||
>**Info:** You can also use [Oracle JDK](https://www.oracle.com/java/technologies/javase-downloads.html). Set the JAVA_HOME environment variable to the pathname of the directory into which you installed JDK. | ||
* [Oracle](https://www.oracle.com/java/technologies/downloads/) |
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.
Is it okay to suggest oracle distributions in our README files (as it can be misleading for our own internal users within WSO2)? @keizer619 WDYT?
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.
Yes Lets suggest Open JDK
Kudos, SonarCloud Quality Gate passed! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 1.8.x #1550 +/- ##
============================================
+ Coverage 79.63% 81.43% +1.80%
- Complexity 1767 1964 +197
============================================
Files 124 124
Lines 10082 11184 +1102
Branches 1670 2110 +440
============================================
+ Hits 8029 9108 +1079
- Misses 1471 1479 +8
- Partials 582 597 +15 ☔ View full report in Codecov by Sentry. |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
Closed PR due to inactivity for more than 18 days. |
Kudos, SonarCloud Quality Gate passed! |
Closed PR due to inactivity for more than 18 days. |
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
Close this PR since we checking the update of gralvm compatibility and resend the needful |
Purpose