DOCS MATTER! #951

xuhang57 · 2018-02-01T18:59:41Z

Trying to fix ALL docs issue. Any effort on reviewing ongoing changes would be much appreciated.

Starting from:

~~- [ ] #648~~

xuhang57 · 2018-02-05T20:14:51Z

For #648, I don't know anything about multi-processes in HIL. Any idea how could I proceed? Or should someone else do it?

naved001

Thanks a lot for taking this up!

I think it would be nice to see if the commands that we are telling people to issue when installing HIL are consistent. I pointed out one instance; we tell them to become root yet still run the subsequent commands with sudo (which is not needed). I think there might be more instances, do you mind just having a look at it while you are at it?

naved001 · 2018-02-06T15:13:41Z

AUTHORS

 Logan Bernard <bernard.logan4@gmail.com>
+Lucas Xu <xuh@massopen.cloud>
+Naved Ansari <naved001@massopen.cloud>


I don't have that email address. you could put my gmail or bu email, it's naved001@gmail.com/bu.edu

free like everybody is not using their regular email addresses here. You sure you want to use the gmail? Why not get a massopen.cloud email from Rado just saying ;)

Not sure I follow the point about "everybody is not using their regular email addresses here". I only see three massopen.cloud addresses, and all of them are introduced by this patch.

Sorry for the confusion. I meant many people are using the email addresses that they don't check on a regular basis. And IMHO, hardcoded email addresses is very bad since it is very easy to be scraped and receive spams.

naved001 · 2018-02-06T15:20:01Z

docs/INSTALL.rst

@@ -16,11 +16,11 @@ Prerequisite Software
 HIL requires a number of packages available through the CentOS RPM
 repositories, as well as the EPEL repository. EPEL can be enabled via::

-    yum install epel-release
+    $ yum install epel-release


oh boy, you went through all the trouble of adding the $ sign everywhere. Sorry I wasn't paying attention that you were taking this up. But if you think about it putting the dollar sign kinda makes it harder to copy and paste it. Earlier, I could triple click and copy the whole line and paste it into a terminal, but with this I would have to remove the dollar sign each time. :(

Don't make this change yet, maybe I copy paste wrong and what you are doing is right. I want to hear from other reviewers or whoever suggested putting the dolla sign there.

Don't make this change yet,

I mean, don't undo your changes yet.

hah, thought we had a conclusion on that issue.
@shwsun ping~

I don't quite understand the debate. It is convention to have a $ denote that it is a command rather something that you should put into a file. We need to distinguish things should be in the config file from commands we want users to run.

@xuhang57 thank you for your time to clean the HIL doc. While you are at it, could you fix this too? It annoys me a while.

@shwsun sure, "user needS" and export before each line? That's all right?
@naved001 once we have a conclusion, I will change the docs accordingly.
ping @zenhack @Izhmash @SahilTikale

I don't quite understand the debate. It is convention to have a $ denote that it is a command rather something that you should put into a file.

I guess I am not a fan of that convention then, for the reasons I mentioned in my comment.

We need to distinguish things should be in the config file from commands we want users to run.

I don't know, but I feel it's apparent from the context and that the fact that we mention on top of the block of lines if it goes into a file.

In any case, I'll wait to hear what others have to say; but now I am inclined to follow the convention just because that's the convention.

@Izhmash @zenhack @SahilTikale @knikolla anyone want to weigh in?

No strong opinion, but my inclination is to include the $, as these are meant to be instructions, not a script -- we should be optimizing for understanding, not copy & paste. The $ is conventional and provides a useful cue to the reader, even if we do mention above that it's a shell command.

I am used to seeing $ when reading documentation. It helps when you need to quickly scan through a wall of text to piece out what needs to be run in the terminal.

naved001 · 2018-02-06T15:20:53Z

docs/INSTALL.rst

+    $ cd /root
+    $ git clone https://github.com/CCI-MOC/hil
+    $ cd hil
+    $ sudo python setup.py install


if you are root at this point, you don't need to sudo.

true. But how could you even cd /root if you a not a root user? hmm.

nvm. sudo su-

Related to the convention discussion above, A common convention is to display root prompts with # rather than $, which I think is a good idea if we go with the prompt indicators.

zenhack

A few comments.

zenhack · 2018-02-06T19:49:32Z

AUTHORS

 Andrew Mohn <AndrewMohn@gmail.com>
 George Silvis, III <george.iii.silvis@gmail.com>
+Ian Ballou ianballou67@gmail.com


Missing brackets.

zenhack · 2018-02-06T19:56:06Z

docs/USING.rst

+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+
+Eg> Node name: mocknode01


This doesn't render particularly well; all of the fields get wrapped on to one line. Also the Eg> notation is weird to me.

Hmm, I am trying to follow the previous examples. And we actually have two different formats in those two existing examples. I am gonna reformat them then and try to come up with one clean format.

I hadn't noticed it was existing notation, but yeah, let's improve upon it.

zenhack · 2018-02-06T20:01:36Z

hil/client/README.md

@@ -0,0 +1,24 @@
+### HIL Python Client API


Start with level 1 headings (one #)

This should go under docs somewhere.

Probably should call it "client library"

This could use some introductory verbiage; You're kindof just plopping some code down without any overall context. Also, we don't describe the design of the library at all; it would be good to briefly describe the important objects and sketch its structure (docstrings can serve as comprehensive reference docs, but we should give an overview).

This should go under docs somewhere.

Personally, I don't like putting this under /docs since there are too many docs there already and people probably won't read it. So I put it in the client directory. But if it is the best practice then I will move it there. Maybe creating a new folder called /client?

Point 1 & 3 sound good to me.

As for the point 4, I thought we have the overview description when the client library first got introduced to the code base. Will discuss with @naved001 and try to come up with an overview. The idea sounds good to me, but I don't have a plan on how to write one atm.

Re: /docs, this is where all of our high-level docs have gone. Maybe we should update the README at the root of the repo to provide a more thorough index, if we want stuff to be easier to find.

I'm looking at the client library as something that people who don't hack on HIL itself might hypothetically use; having it buried inside of the source code doesn't make much sense. It might be more reasonable if this were an internal-use-only module.

Re (4): if we have overview docs, I don't know where they are.

…eanup

xuhang57 · 2018-02-08T16:13:59Z

@zenhack pushed a brief description of the client library. Please let me know if you think it needs to be improved. 👍

@shwsun @naved001 done.

ianballou

I think it's looking good, just one fix and a comment. Also we'll want to make sure the $ convention is consistent though all the documentation if we decide to change to that.

ianballou · 2018-02-08T18:15:55Z

docs/client-library.md

@@ -0,0 +1,28 @@
+# HIL Client Library
+
+##Description


You need a space here between the ## and Description

ianballou · 2018-02-08T18:34:07Z

docs/client-library.md

+```
+pip install git+https://github.com/cci-moc/hil
+```
+


We might want to document which client library commands are available if we don't want users to have to look through the source.

xuhang57 · 2018-02-12T16:35:21Z

@zenhack @naved001 done.

…eanup

…ocs-cleanup

ianballou

Alright, looks good to me!

zenhack

One minor thing, otherwise I'm happy.

zenhack · 2018-02-13T20:34:47Z

docs/USING.rst

+   - **Switch type:** mock
+   - **Host name:**   switchhost01
+   - **User name:**   switchuser01
+   - **Password:**    password1234


This is currently being rendered as a block comment; you should un-indent it. There are a couple of other instances of this problem elsewhere.

Was doing that on purpose since that's what one of the examples did previously. Hmm, I think you ar right and it doesn't make sense to block comment them.

xuhang57 · 2018-02-14T15:41:15Z

@zenhack done.

zenhack · 2018-02-15T23:24:54Z

Great, merging.

WIP: DOCS MATTER!

0fe8943

xuhang57 added waiting on submitter WIP labels Feb 1, 2018

xuhang57 self-assigned this Feb 1, 2018

xuhang57 added cleanup documentation labels Feb 1, 2018

xuhang57 added 5 commits February 1, 2018 14:14

add README to hil client api

b42879b

change the readme in /client

71b6734

add more examples in the cli docs

d47f8cc

fix ident

d320094

fix space

3ba4996

xuhang57 requested a review from naved001 February 2, 2018 16:21

xuhang57 added 3 commits February 2, 2018 11:28

Try to cover everyone

afed5b2

add create a network example

39f630c

trailing space

62b5246

xuhang57 added waiting on reviewer and removed WIP waiting on submitter labels Feb 5, 2018

xuhang57 changed the title ~~WIP: DOCS MATTER!~~ DOCS MATTER! Feb 5, 2018

naved001 reviewed Feb 6, 2018

View reviewed changes

xuhang57 added 2 commits February 6, 2018 15:59

a few changes

b7a0acd

cleanup unused vars

9be2510

zenhack suggested changes Feb 6, 2018

View reviewed changes

Update AUTHORS

8db62da

xuhang57 added waiting on submitter and removed waiting on reviewer labels Feb 6, 2018

xuhang57 added 3 commits February 7, 2018 19:43

change the format

8742d15

moving client library docs to /docs

3f81713

add client library md

57232e5

xuhang57 added 2 commits February 8, 2018 15:25

Merge branch 'master' of https://github.com/xuhang57/hil into docs-cl…

93db2e5

…eanup

add description for Client Library

4af5a61

xuhang57 added waiting on reviewer and removed waiting on submitter labels Feb 8, 2018

ianballou reviewed Feb 8, 2018

View reviewed changes

xuhang57 added 2 commits February 8, 2018 13:51

Update client-library.md

ddf555d

Update client-library.md

18be774

xuhang57 added 2 commits February 12, 2018 16:42

Merge branch 'master' of https://github.com/xuhang57/hil into docs-cl…

160b066

…eanup

Merge branch 'docs-cleanup' of https://github.com/xuhang57/hil into d…

bee014a

…ocs-cleanup

ianballou approved these changes Feb 13, 2018

View reviewed changes

zenhack suggested changes Feb 13, 2018

View reviewed changes

xuhang57 added 2 commits February 14, 2018 10:37

Update USING.rst

955f144

Update USING.rst

55e6730

xuhang57 added the waiting on +2 label Feb 14, 2018

zenhack approved these changes Feb 15, 2018

View reviewed changes

zenhack merged commit b2fdec6 into CCI-MOC:master Feb 15, 2018

xuhang57 deleted the docs-cleanup branch February 16, 2018 14:33

xuhang57 mentioned this pull request Feb 20, 2018

Update License and Authors informations #710

Closed

DOCS MATTER! #951

DOCS MATTER! #951

Conversation

xuhang57 commented Feb 1, 2018 • edited Loading

xuhang57 commented Feb 5, 2018

naved001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xuhang57 Feb 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zenhack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xuhang57 Feb 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xuhang57 commented Feb 8, 2018

ianballou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xuhang57 commented Feb 12, 2018

ianballou left a comment

Choose a reason for hiding this comment

zenhack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xuhang57 commented Feb 14, 2018

zenhack commented Feb 15, 2018

xuhang57 commented Feb 1, 2018 •

edited

Loading

xuhang57 Feb 6, 2018 •

edited

Loading

xuhang57 Feb 6, 2018 •

edited

Loading