-
-
Notifications
You must be signed in to change notification settings - Fork 118
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
Update adoptium links #94
Conversation
https://github.com/onthegomap/planetiler/actions/runs/1933271845 ℹ️ Base Logs 714c636
ℹ️ This Branch Logs 3729d75
|
CONTRIBUTING.md
Outdated
@@ -3,7 +3,7 @@ | |||
Pull requests are welcome! To set up your development environment: | |||
|
|||
- Fork the repo | |||
- [Install Java 16 or later](https://adoptium.net/installation.html) | |||
- Install Java 16 or later. You can download Java manually from [Adoptium](https://adoptium.net/installation.html) or use their installers for [Windows](https://adoptium.net/installation.html#windows-msi), [macOS](https://adoptium.net/installation.html#macos-pkg), and [Linux](https://github.com/adoptium/website-v2/blob/main/src/asciidoc-pages/installation/linux.adoc). |
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 install url is also referenced from planetiler-examples/README.md
and README.md
https://github.com/onthegomap/planetiler/search?q=https%3A%2F%2Fadoptium.net%2Finstallation.html - think we should change it there as well?
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.
Good catch. I suggest cross-referencing the CONTRIBUTING.md file, see commit
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.
Thanks! that looks good. What do you think about inlining the command for common installers?
brew install --cask temurin
apt-get install openjdk-17-jdk
(probably better to prefer the recommended distro's openjdk build)
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.
Sounds like a good idea
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.
Cool, do you want to add those or want me to?
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.
Best if you do it. Do you have write access on my branch?
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.
Looks like I can update your branch - how does this look?
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.
Looks good to me!
Expands adoptium java install links to directly point to win, mac, and linux installers.