-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[DOCS-5624] inclusive naming screenshot update inbound agent #5209
Conversation
preview deploy link, https://deploy-preview-5209--jenkins-io-site-pr.netlify.app/doc/book/scaling/architecting-for-scale/#agent-communications updated the image and text just prior to the image to use agent instead of slave. The links in use on this page are still using "slave" in their url string, so these cannot be updated without breaking, and would need to be updated by their maintainers/owners respectively. |
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.
LGTM!
Thanks! Nice improvements. Can you confirm on a Windows computer that the revised instructions work? I do not want to merge this unless we've confirmed that the instructions work as listed in the new text. Before merging, I would also like to replace the use of "JNLP" with "inbound". The "JNLP" acronym tends to confuse users because it is associated with the Java Web Start feature that was removed from Java 9. |
Good change and a nice step forward. It needs one more change. The term "JNLP" is deprecated because it is an inaccurate representation of connection technique the agent is using. They are now called "inbound agents" rather than "JNLP agents". This may be a good one for us to work together during a one on one. |
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.
A great improvement. Thanks. Before we merge, I think we should extend the improvement to replace the "JNLP" terminology with the current terminology "inbound" or "inbound agent".
Related: jenkinsci/docker-inbound-agents#31 |
@@ -141,8 +141,8 @@ also terminology here is old (s/JNLP/inbound/g), and WebSocket should be mention | |||
firewalled network connecting to a controller located outside the firewall. | |||
|
|||
. As a service: First you'll need to manually launch the agent using the above | |||
method. After manually launching it, _jenkins-slave.exe_ and | |||
_jenkins-slave.xml_ will be created in the agent's work directory. Now go to | |||
method. After manually launching it, _jenkins-agent.exe_ and |
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.
Actually the whole previous paragraph is obsolete as of jenkinsci/jenkins#6543 so even if jenkinsci/windows-slave-installer-module#4 had renamed the exe file—which it did not—it would no longer matter.
You can still set up a Windows service for agents, I suppose, but instructions would be different now. You would need to find a winsw expert for details.
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.
Hi @jglick thanks for the insight, I was not aware of that prior. Would you happen to know if there is a winsw expert at CloudBees that might be able to help me with this in that case?
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.
Not sure offhand. I think @oleg-nenashev used to maintain this; not sure if he is currently available for advice.
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.
Hi @oleg-nenashev when you have a moment, would you be able to share any insight you have on this process? I want to be sure that when updating the inclusive naming that the surrounding context will still be accurate. Thanks very much!
returning some of the original command/file paths as they have not been updated for inclusive naming
Updated screenshot and terminology to be in line with current standards & usage. Also returned filepaths/commands back to original non-inclusive state as these files may not have been updated by the maintainer. |
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.
Nice improvement. Thanks!
updated image and text within this page to reflect the inclusive naming preference of agent.