This repository has been archived by the owner on Apr 27, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 54
DOCS MATTER! #951
Merged
Merged
DOCS MATTER! #951
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
0fe8943
WIP: DOCS MATTER!
xuhang57 b42879b
add README to hil client api
xuhang57 71b6734
change the readme in /client
xuhang57 d47f8cc
add more examples in the cli docs
xuhang57 d320094
fix ident
xuhang57 3ba4996
fix space
xuhang57 afed5b2
Try to cover everyone
xuhang57 39f630c
add create a network example
xuhang57 62b5246
trailing space
xuhang57 b7a0acd
a few changes
xuhang57 9be2510
cleanup unused vars
xuhang57 8db62da
Update AUTHORS
xuhang57 8742d15
change the format
xuhang57 3f81713
moving client library docs to /docs
xuhang57 57232e5
add client library md
xuhang57 93db2e5
Merge branch 'master' of https://github.com/xuhang57/hil into docs-cl…
xuhang57 4af5a61
add description for Client Library
xuhang57 ddf555d
Update client-library.md
xuhang57 18be774
Update client-library.md
xuhang57 160b066
Merge branch 'master' of https://github.com/xuhang57/hil into docs-cl…
xuhang57 bee014a
Merge branch 'docs-cleanup' of https://github.com/xuhang57/hil into d…
xuhang57 955f144
Update USING.rst
xuhang57 55e6730
Update USING.rst
xuhang57 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, don't undo your changes yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hah, thought we had a conclusion on that issue.
@shwsun ping~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I am not a fan of that convention then, for the reasons I mentioned in my comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Izhmash @zenhack @SahilTikale @knikolla anyone want to weigh in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.