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

Fix doc #10619

Merged
merged 4 commits into from Aug 15, 2016
Merged

Fix doc #10619

merged 4 commits into from Aug 15, 2016

Conversation

chengweiv5
Copy link
Contributor

No description provided.

@@ -265,7 +265,7 @@ scrubbing operations.

``osd scrub min interval``

:Description: The maximum interval in seconds for scrubbing the Ceph OSD Daemon
:Description: The minimal interval in seconds for scrubbing the Ceph OSD Daemon
Copy link
Contributor

Choose a reason for hiding this comment

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

could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#preparing-and-sending-patches

in this case, it would be "doc: "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tchaikov , of course, thanks


rbd map mypool/myimage --cluster *cluster name*
rbd map mypool/myimage --cluster cluster name
Copy link
Contributor

Choose a reason for hiding this comment

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

@chengweiv5 could you change it to cluster-name and change the --cluster cluster name to --cluster cluster-name accordingly?

cluster name might confuse user: there are two words.

@tchaikov
Copy link
Contributor

other than s/cluster name/cluster-name/, lgtm.

@chengweiv5
Copy link
Contributor Author

@tchaikov cluster name shows up 6 times in this document, should I change all of them to cluster-name?

@tchaikov
Copy link
Contributor

@chengweiv5 no, just the one in Synopsis section and the one in Options section (--cluster cluster name), and the one you are changing.

@chengweiv5
Copy link
Contributor Author

@tchaikov ok, I'll update it.

Signed-off-by: Chengwei Yang <yangchengwei@qiyi.com>
@@ -56,14 +56,14 @@ To list the cluster's keys and their capabilities, execute the following::
Placement Group Subsystem
=========================

To display the statistics for all placement groups, execute the following::
To display the statistics for all placement groups, execute the following::
Copy link
Contributor

Choose a reason for hiding this comment

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

@chengweiv5, does this fix a formatting problem in the output or silence a warning of sphinx?

Copy link
Contributor

Choose a reason for hiding this comment

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

if not, i'd recommend accompany the kill-trailing-spaces only changes with some more significant change.

Signed-off-by: Chengwei Yang <yangchengwei@qiyi.com>
Signed-off-by: Chengwei Yang <yangchengwei@qiyi.com>
Signed-off-by: Chengwei Yang <yangchengwei@qiyi.com>
@chengweiv5
Copy link
Contributor Author

updated, do not remove trailing space

@@ -329,7 +329,7 @@ The ``quorum`` list at the end lists monitor nodes that are part of the current

This is also available more directly::

$ ./ceph quorum_status
ceph quorum_status
Copy link
Contributor

Choose a reason for hiding this comment

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

could you keep the $? this is an example of command usage with its output. the idea is to show the interaction of input and output. so $ helps to differentiate the command line from its output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but this isn't consistent with other commands, you see there are below 3 commands, all of them without $

ceph-mon-stat

ceph-mon_status

ceph-mon-dump

Copy link
Contributor

Choose a reason for hiding this comment

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

okay!

@tchaikov tchaikov merged commit f468c73 into ceph:master Aug 15, 2016
@tchaikov tchaikov self-assigned this Aug 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants