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

ISPN-13981: Restore the Enabling JMX remote ports module on the 13.0.x branch #10201

Merged
merged 1 commit into from Jul 13, 2022

Conversation

dfitzmau
Copy link
Contributor

@dfitzmau dfitzmau added the Documentation Pull request containing only documentation changes label Jun 30, 2022
@dfitzmau dfitzmau force-pushed the ISPN-13981 branch 2 times, most recently from 6e70e78 to 3edd09b Compare June 30, 2022 16:00
Copy link
Member

@domiborges domiborges left a comment

Choose a reason for hiding this comment

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

@dfitzmau The file is missing conditionals and introduces several blank lines. Please copy the file from main

@dfitzmau
Copy link
Contributor Author

dfitzmau commented Jul 1, 2022

Hello @tristantarrant . Would you be OK to review this PR for the restored file?

ifdef::remote_caches[]
[NOTE]
====
{brandname} Server does not expose JMX remotely by using the single port endpoint. If you want to remotely access the {brandname} Server through JMX, you must enable a remote port.
Copy link
Member

Choose a reason for hiding this comment

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

Each sentence should start on its own line

Suggested change
{brandname} Server does not expose JMX remotely by using the single port endpoint. If you want to remotely access the {brandname} Server through JMX, you must enable a remote port.
{brandname} Server does not expose JMX remotely by using the single port endpoint.
If you want to remotely access the {brandname} Server through JMX, you must enable a remote port.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @dvagnero .

I updated the content based on your feedback.

Copy link
Member

@domiborges domiborges left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the improvements

@tristantarrant
Copy link
Member

Suggestion: we could add a link to the Java docs that show how to configure security https://docs.oracle.com/en/java/javase/11/management/monitoring-and-management-using-jmx-technology.html#GUID-F08985BB-629A-4FBF-A0CB-8762DF7590E0

@dfitzmau
Copy link
Contributor Author

dfitzmau commented Jul 6, 2022

This could be a good idea, but referring to external docs in our downstream docs might have legal issues. I don't see any issue with the upstream referral, but downstream maybe.

@dvagnero , would you usually refer to external docs in this instance?

@tristantarrant
Copy link
Member

We already link to external doc sources (e.g. JDK Javadocs)

@dfitzmau
Copy link
Contributor Author

dfitzmau commented Jul 7, 2022

Thanks, @tristantarrant .

I provided the link in the context of the upstream docs. Downstream might be trickier, so I added an if-def statement. I hope this is OK?

@domiborges domiborges merged commit 0329d99 into infinispan:13.0.x Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Pull request containing only documentation changes
Projects
None yet
3 participants