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

please clarify documentation on service watch #40819

Closed
shallot opened this issue Apr 21, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@shallot
Copy link

commented Apr 21, 2017

The document at https://docs.saltstack.com/en/latest/ref/states/all/salt.states.service.html is too confusing for its own good when it comes to the watching. The description at salt.states.service.mod_watch doesn't go into any nuance, it just says:

"The service watcher, called to invoke the watch command."

If the user looks that up, they will miss some of the nuance from the general section:

"By default if a service is triggered to refresh due to a watch statement the service is by default restarted."

This sentence should be moved or copied under mod_watch.

Also, the term "by default" is repeated twice in the same sentence, so one of those should be removed.

Furthermore, the descriptions of options are problematic:

"Use reload instead of the default restart (exclusive option with full_restart, defaults to reload if both are used)"

Please rephrase the part in parenthesis, it sounds like it's trying to map boolean logic into simple English, but it's not actually succeeding, since the phrase "Y = Exclusive option with X" is a bad translation of XOR - just say "Y cannot be combined with X". Pseudo-code would have been more readable than this :)

"Use service.full_restart instead of restart (exclusive option with reload)"

Ditto for the parenthesized part.

In context of a state named "service", the reference to "service.full_restart", which I assume is to the module named "service", is unclear. Especially when one can't actually find that at https://docs.saltstack.com/en/latest/ref/modules/all/salt.modules.service.html

Also, somewhere in there it wouldn't hurt to explain briefly what is meant by "full" restart - someone might think it's service(8)'s --full-restart, but it could also be assumed to be the module running a stop followed by a start itself, or maybe some third option I'm not currently acquainted with.

Please fix this. TIA.

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented Apr 21, 2017

Would you mind helping us clean up these documents?

Sometimes we are far to familiar with what is going on underneath that we are unable to write clear documentation and would greatly appreciate assistance in cleaning them up.

Thanks!
Daniel

@gtmanfred gtmanfred added this to the Approved milestone Apr 21, 2017

@stale

This comment has been minimized.

Copy link

commented Oct 6, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Oct 6, 2018

@shallot

This comment has been minimized.

Copy link
Author

commented Oct 8, 2018

This all still applies

@stale

This comment has been minimized.

Copy link

commented Oct 8, 2018

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Oct 8, 2018

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2018

@MTecknology You've been knocking out some of these documentation discrepancies lately. Perhaps you'd like to help out here as well?

@MTecknology

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2018

@shallot Feel free to review, particularly the bit about what mod_watch() is.

@rallytime rallytime closed this in 05d9331 Oct 9, 2018

rallytime added a commit that referenced this issue Oct 9, 2018

Merge pull request #49940 from MTecknology/docs-fix-40819
Clean up documentation/language in service state. (Fixes #40819)

gitebra pushed a commit to gitebra/salt that referenced this issue Oct 10, 2018

Merge remote-tracking branch 'upstream/develop' into develop
* upstream/develop: (54 commits)
  Fix quoting issue when passing extra_vars to ansible-playbook
  Add a docstring
  Add UTF-8 encoding
  Update cloud_controller.rst
  Update mod_watch() description for all states.
  Another attempt to clean up service.mod_watch description.
  Add comment to change for Windows
  Add some doc tests to the filemap that should run on every PR
  Clean up documentation/language in service state. (Fixes saltstack#40819)
  Remove support for RAET
  Add salt-support to the setup.py
  Make profiles a package.
  Update cloud_controller.rst
  Fix py2/3 compatibility issue
  Fix bug in gem.installed related to rvm
  Fix issues with startup in Windows
  Adding in some missing excepts to handle when an invalid type is configured for the master.
  Adding a test.
  If ipv6 is enabled and it fails, try the lookup again using ipv4 just in case the master is a ipv4 address.
  Skip ansiblegate state integration test on CentOS 6
  ...

rallytime added a commit to rallytime/salt that referenced this issue Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.