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

Mention full JDK as a prerequisite #3340

Closed
wants to merge 1 commit into from
Closed

Mention full JDK as a prerequisite #3340

wants to merge 1 commit into from

Conversation

khinsen
Copy link

@khinsen khinsen commented May 6, 2023

CIDER apparently needs a full JDK (see #3339), rather than just a Java runtime. This is not obvious, so it should be stated explicitly in the list of prerequisites.


Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
    (the remaining items do not apply as only the documentation is modified).

Copy link
Member

@vemv vemv left a comment

Choose a reason for hiding this comment

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

Thanks for finding this missing spot!

Comment on lines +23 to +24
Make sure you have a complete JDK installation. Many Linux distributions
provide only the runtime support by default.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Make sure you have a complete JDK installation. Many Linux distributions
provide only the runtime support by default.
On Linux you are also strongly advised to make sure that JDK sources and javadocs are installed.
You can find example commands in xref:troubleshooting.adoc#navigation-to-jdk-sources-doesnt-work[Troubleshooting].

I took that from https://docs.cider.mx/cider/about/compatibility.html#java . Probably it's a good idea so simply have the same paragraph/link twice.

Copy link
Member

Choose a reason for hiding this comment

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

Generally the full JDK is needed only if people are using cider-nrepl, but I guess that's most of the users anyways.

@bbatsov
Copy link
Member

bbatsov commented May 17, 2023

So, what will we have here as the final wording?

@khinsen
Copy link
Author

khinsen commented May 18, 2023

I'd prefer something that is clear to newcomers, who are the most in need of installation instructions. That's people who know very little about CIDER and probably even nREPL. When I read "strongly advised to make sure that JDK sources and javadocs are installed", I am left wondering why JDK sources and javadocs matter for CIDER, given that they are "strongly advised" but not required.

If you want to send the message that experienced users should investigate this for themselves, then I'd write "Unless you know what you are doing, install a full JDK with sources and javadocs".

@vemv vemv closed this in 8023414 May 18, 2023
@vemv
Copy link
Member

vemv commented May 18, 2023

Thanks for the input!

I changed strongly advised to required. Which sounds slightly stringent, but it will leave things clear beyond any doubt.

Unless you know what you are doing, install a full JDK with sources and javadocs

I considered this one but it kind of invites adventurous devs to try random things? 😄

r0man pushed a commit to r0man/cider that referenced this pull request Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants