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

Heartbeat Automatic Reloading Docs #8227

Merged
merged 3 commits into from
Sep 21, 2018
Merged

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Sep 5, 2018

Docs for the changes in #8023 for heartbeat automatic reloading.

@andrewvc andrewvc added docs in progress Pull request is currently in progress. Heartbeat labels Sep 5, 2018
return fmt.Sprintf("Monitor not loaded, plugin is disabled")
}

type MonitorPluginInfo struct {

Choose a reason for hiding this comment

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

exported type MonitorPluginInfo should have comment or be unexported

"github.com/elastic/beats/libbeat/common"
)

type PluginDisabledError struct{}

Choose a reason for hiding this comment

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

exported type PluginDisabledError should have comment or be unexported

}
}

func (m *Monitor) Stop() {

Choose a reason for hiding this comment

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

exported method Monitor.Stop should have comment or be unexported

return nil
}

func (m *Monitor) Start() {

Choose a reason for hiding this comment

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

exported method Monitor.Start should have comment or be unexported

@andrewvc andrewvc changed the title [WIP] Heartbeat Automatic Reloading Docs Heartbeat Automatic Reloading Docs Sep 15, 2018
@andrewvc andrewvc added review and removed in progress Pull request is currently in progress. labels Sep 15, 2018
@urso urso mentioned this pull request Sep 17, 2018
12 tasks
determine if they are available. Each `monitor` item begins with a dash (-) and
specifies the type of monitor to use, the hosts to check, and other settings
that control Heartbeat behavior.
Heartbeat is configured by defining a set of `monitors` to check your remote hosts.
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that you've changed this section to be more task-focused, but you should avoid passive voice, and speak directly to the user. I'd suggest starting with a description of what a monitor is and then explain how to configure them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just an FYI that we should use asciidoc attributes to resolve product names, so use {beatname_uc} instead of Heartbeat. I try to do this whenever I update topics these days.

specifies the type of monitor to use, the hosts to check, and other settings
that control Heartbeat behavior.
Heartbeat is configured by defining a set of `monitors` to check your remote hosts.
These can be specified either inside the +heartbeat.yml+ config file, or in external
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I would avoid passive voice.

Copy link
Contributor

Choose a reason for hiding this comment

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

I want to tighten up the language used here, but rather than entering comments on this PR, it'll be more efficient for me to come back later and edit these additions in a separate PR. So for now, I'll just point out stuff that is potentially confusing, rather than style issues. Created an issue here: #8332


experimental[]

THIS EXPERIMENTAL FEATURE IS NOW DEPRECATED. IT WILL BE REMOVED IN A FUTURE
Copy link
Contributor

@dedemorton dedemorton Sep 17, 2018

Choose a reason for hiding this comment

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

You should replace the experimental[] tag with deprecated (see https://github.com/elastic/docs#section-notifications). Within the tag itself, you can add details to provide more info about the deprecation.

@andrewvc
Copy link
Contributor Author

@dedemorton I believe I've addressed the issues you mentioned with beat name references and the passive voice in the last two commits. LMK what you think :)

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewvc andrewvc merged commit 04597c8 into elastic:master Sep 21, 2018
andrewvc added a commit to andrewvc/beats that referenced this pull request Sep 21, 2018
Docs for the changes in elastic#8023 for heartbeat automatic reloading.

(cherry picked from commit 04597c8)
andrewvc added a commit that referenced this pull request Sep 27, 2018
Docs for the changes in #8023 for heartbeat automatic reloading.

(cherry picked from commit 04597c8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants