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

[DOCS-5624] inclusive naming screenshot update inbound agent #5209

Merged
merged 4 commits into from
Jul 22, 2022
Merged

[DOCS-5624] inclusive naming screenshot update inbound agent #5209

merged 4 commits into from
Jul 22, 2022

Conversation

kmartens27
Copy link
Contributor

updated image and text within this page to reflect the inclusive naming preference of agent.

@probot-autolabeler probot-autolabeler bot added the documentation Jenkins documentation, including user and developer docs, solution pages, etc. label Jun 9, 2022
@kmartens27
Copy link
Contributor Author

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.

Copy link
Contributor

@cari-mason cari-mason left a comment

Choose a reason for hiding this comment

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

LGTM!

@kmartens27 kmartens27 marked this pull request as ready for review June 9, 2022 15:48
@kmartens27 kmartens27 requested a review from a team as a code owner June 9, 2022 15:48
@MarkEWaite
Copy link
Contributor

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.

@MarkEWaite
Copy link
Contributor

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.

Copy link
Contributor

@MarkEWaite MarkEWaite left a 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".

@hervelemeur
Copy link

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
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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!

@kmartens27 kmartens27 changed the title [DOCS-5624] inclusive naming update jnlp agent [DOCS-5624] inclusive naming screenshot update inbound agent Jul 7, 2022
kmartens27 and others added 2 commits July 22, 2022 12:38
returning some of the original command/file paths as they have not been updated for inclusive naming
@kmartens27
Copy link
Contributor Author

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.

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Nice improvement. Thanks!

@MarkEWaite MarkEWaite merged commit 3b2978f into jenkins-infra:master Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Jenkins documentation, including user and developer docs, solution pages, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants