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

WFCORE-5841 [primary/secondary] Review the documentation of affected CLI management operations #5098

Merged
merged 1 commit into from May 19, 2022

Conversation

moulalis
Copy link
Contributor

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label May 10, 2022
@@ -46,8 +46,8 @@ ARGUMENTS
when it restarts. An ADMIN_ONLY controller will start any
configured management interfaces and accept management
requests, but will not start servers or, if this host
controller is the master for the domain, accept incoming
connections from slave host controllers. For embedded host
controller is the primary for the domain, accept incoming
Copy link
Collaborator

Choose a reason for hiding this comment

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

Our analysis doc states the following:

"Where ‘master’ was used as a shorthand for ‘master Host Controller’ replace with either ‘Domain Controller’ or ‘primary Host Controller’."

So I think we should use ‘primary Host Controller’ here. @bstansberry Would you mind confirming? honestly, I always have difficulties with these kinds of subtlety.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here, waiting for @bstansberry to confirm.

Copy link
Contributor

Choose a reason for hiding this comment

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

@moulalis @yersan +1 for "primary Host Controller"

The way it is in the PR now is fine in terms of being correct English and being clear to the reader. But I think it will be easier for future maintainers whose 'primary' ;-) skill is writing code and not writing English if we follow the KISS principle and avoid shorthand.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@moulalis Could you please update the PR accordingly? thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR updated. Thanks.

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label May 19, 2022
@yersan yersan merged commit 3eeb37b into wildfly:main May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
3 participants