-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update documentation #74
base: main
Are you sure you want to change the base?
Conversation
- Updated the "man page" section starting at line 414 to make the documentation more accurate, readable, and useful. - Fixed some grammar issues
Arguments: | ||
--source: (Optional) Specifies the source directory for modules (defaults to the Crowdbotics modules directory based on package location). | ||
--project: (Optional) Specifies the project directory to install modules into (defaults to the git root of the current working directory). | ||
Parameters: | ||
<module1> <module2>: Names of the modules to install (required). | ||
Example: cb add module1 module2 |
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.
Nit pick on the language used here.
First let's look at the entire command:
cb add module1 module2 --source something --project foobar
That's 8 arguments as far as the shell is concerned (space is the separator), and one can access them in bash via $0
through $7
.
First argument is cb
, second is add
, the last is foobar
, etc.
But when it comes to building man pages, we usually call the various arguments differently depending on their syntax and position - they can be called "command", "subcommand", "argument" (or positional argument), "parameter" and "option".
With that in mind, here's a few examples:
cb add module1 module2 --source something --project foobar
^^
command
cb add module1 module2 --source something --project foobar
^^^
subcommand
cb add module1 module2 --source something --project foobar
^^^
argument or positional argument
cb add module1 module2 --source something --project foobar
^^^
parameter (parameter with value "something" for the option "source")
cb add module1 module2 --source something --project foobar --flaglongform
^^^
option (also known as flag and in the long form syntax)
cb add module1 module2 --source something --project foobar -f
^^^
option (also known as flag and in short form syntax)
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.
PS: Most man pages just refer to all arguments as options and the only distinction is syntax. See this git-add man page for example:
https://git-scm.com/docs/git-add
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.
TL;DR - calling everything generically an option is fine, but when you want to be precise we should be consistent in the language.
Also, this is the "help" command output, this shouldn't be an exhaustive reference or man page of all commands. Even man pages are divided by subcommands. Perhaps its best if we put --help
and -h
on all commands and just show the command/options part for each.
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'm fine with any changes you want to make. @codydjango noticed some grammar issues.
- There is a bit of disagreement on the help page. Some folks say the man page should be the complete version and the docs should be just an overview, so users can find everything the need at the command line. Others have suggested, as you have, that the man page shouldn't be exhaustive. Either way, we need to have an exhaustive list somewhere.
- Also, @codydjango did ask for a --help on "cb" as well. Yes, you get the full list if you just type "cb" . But devs are used to the --help functionality.
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.
Yup, I did but I'm willing to defer to @danielsousaio.
Daniel, I did ask Marni to review the docs because I noticed some typos and awkward phrasing. It was a few weeks back, so the worst offenders might have already been addressed.
What do you think? Should we disregard the PR, or are there specific changes that can be done to move this forward?
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.
Others have suggested, as you have, that the man page shouldn't be exhaustive. Either way, we need to have an exhaustive list somewhere.
Yes, I'm suggesting that we make cb --help
or cb help
short and concise. A description, a list of the subcommands, and a few common commands examples.
Then on each subcommand do i.e cb add --help
or cb help add
with full list of all the options available for that command and the details for each.
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.
Should we disregard the PR, or are there specific changes that can be done to move this forward?
I don't think we should disregard the PR, it's a nice start to improving the help docs - I just think it needs some cleanup.
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 can collaborate with you @CrowdboticsDocs here, and implement the "help" per subcommand.
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.
@danielsousaio I totally agree, I think your suggestion is great: a short and concise cb --help
and then each subcommand has it's own --help
with more detail and example. Thanks for following up on this.
get <id>: Gets details for a specific module by its ID (required subcommand with parameter). | ||
archive <id>: Archives a specific module by its ID (required subcommand with parameter). | ||
help: Shows help for the modules command (optional subcommand). | ||
Example: |
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.
we either need to remove this or generate some real examples here. there are a lot of arguments and parameters here.
@daniel Sousa ***@***.***>, I'll leave this to you for now.
Let me know when you're ready for me to re-review the contents of the file.
…On Mon, Apr 15, 2024 at 12:28 PM Cody Redmond ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In index.js
<#74 (comment)>:
> + Arguments:
+ --source: (Optional) Specifies the source directory for modules (defaults to the Crowdbotics modules directory based on package location).
+ --project: (Optional) Specifies the project directory to install modules into (defaults to the git root of the current working directory).
+ Parameters:
+ <module1> <module2>: Names of the modules to install (required).
+ Example: cb add module1 module2
@danielsousaio <https://github.com/danielsousaio> I totally agree, I
think your suggestion is great: a short and concise cb --help and then
each subcommand has it's own --help with more detail and example. Thanks
for following up on this.
—
Reply to this email directly, view it on GitHub
<#74 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFW5LB3GLQOSWYOSST7VFJ3Y5P5ZXAVCNFSM6AAAAABF5DUTPCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMBRGU4DSMJRGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Ticket
DOC-50
Type of PR
Changes introduced
Test and review
Compare to the previous version.