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

Jump to bottom

Update binary documentation in README #1723

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Update binary documentation in README #1723

merged 1 commit into from
Jan 9, 2025

Conversation

datho7561
Copy link
Contributor

  • Do not specify the version of GraalVM to use, since it's likely that the latest GraalVM works, and the lifecycle of GraalVM versions means any version we list will likely be out of date shortly.
  • Remove instructions to use gu install native-image, because in newer versions of GraalVM, gu is no longer included and native-image is included by default

Fixes #1722

@datho7561
Copy link
Contributor Author

@rgrunber if you have a chance, could you please look over these changes? You have a good understanding of the GraalVM build process

rgrunber
rgrunber reviewed Jan 9, 2025
View reviewed changes
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

We use the setup-graalvm workflow, so I dug into whether it's truly safe (goes without saying downloading a recent graalvm jdk does include native-image).

https://github.com/graalvm/setup-graalvm/blob/4a200f28cd70d1940b5e33bd00830b7dc71a7e2b/src/gu.ts#L45

Seems pretty safe based on that (and the PR referenced).

README.md Outdated Show resolved Hide resolved
@datho7561
Update binary documentation in README
635834b 
- Do not specify the version of GraalVM to use,
  since it's likely that the latest GraalVM works,
  and the lifecycle of GraalVM versions means
  any version we list will likely be out of date shortly.
- Remove instructions to use `gu install native-image`,
  because in newer versions of GraalVM,
  `gu` is no longer included and `native-image` is included by default

Fixes eclipse-lemminx#1722

Signed-off-by: David Thompson <[email protected]>
@datho7561 datho7561 force-pushed the 1722-readme-binary-docs branch from 508acc0 to 635834b Compare January 9, 2025 16:37
@datho7561 datho7561 mentioned this pull request Jan 9, 2025
Closed
@datho7561 datho7561 merged commit 1f65a45 into eclipse-lemminx:main Jan 9, 2025
4 of 6 checks passed
@datho7561 datho7561 deleted the 1722-readme-binary-docs branch January 9, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgrunber rgrunber rgrunber approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update documentation in README for building a binary of lemminx
2 participants
@datho7561 @rgrunber