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

Update Documentations #263

Merged
merged 9 commits into from
Nov 11, 2018
Merged

Update Documentations #263

merged 9 commits into from
Nov 11, 2018

Conversation

prokarius
Copy link
Collaborator

@prokarius prokarius commented Nov 10, 2018

Add list of params to functions in UG
Fix Issue #204

@prokarius prokarius added the Documentation Documentation for repo label Nov 10, 2018
@prokarius prokarius added this to the v1.4 milestone Nov 10, 2018
@prokarius prokarius self-assigned this Nov 10, 2018
@coveralls
Copy link

coveralls commented Nov 10, 2018

Pull Request Test Coverage Report for Build 856

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.122%

Totals Coverage Status
Change from base Build 854: 0.0%
Covered Lines: 2498
Relevant Lines: 2654

💛 - Coveralls

@prokarius prokarius changed the title Add list of params to functions in UG WIP Update Documentations Nov 10, 2018
@prokarius prokarius changed the title WIP Update Documentations Update Documentations Nov 10, 2018
`e/`: `PERSON'S EMAIL` +
`b/`: `BIKE NAME` to be rented +
`lr/`: `RATE` of the loan, in dollars per hour +
[`t/`: `TAGS`] (Optional) +
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part is not formatted consistently with the command above (addbike). May I suggest standardising it to
n/NAME: The customer's name.
[t/TAGS]: Optional tags to tag the loan with.
etc.?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is OK, because it tells you the value of what each prefix is expecting though.
The addbike one had:
n/: BIKE NAME, which is consistent to this one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should not format things like BIKE NAME, since it's not a code term and especially if it's inconsistent with the name used in the Format specification (like e/EMAIL in the Format specs vs e/:PERSON'S EMAIL in the Parameters specs). I think it's good to separate the technical names from the explanations as well (e.g. make a distinction between Bike and bike; one is a technical name, the other is plain English). Hence my suggestion, to make things clearer by using similar conventions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, you raise a good point. I have fixed this issue.

docs/UserGuide.adoc Outdated Show resolved Hide resolved
docs/UserGuide.adoc Outdated Show resolved Hide resolved
@@ -217,12 +257,19 @@ Examples:
Set the password of the app to `n3wP4sS`.
// end::setpass[]

[big red]#List of Parameters#:

The old and new passwords of the application. +
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't follow the format established for the other commands.
Try using:

CURRENT_PASSWORD: The old password of the application.
NEW_PASSWORD: The new password.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't want to do it this way because some people might think that these are prefixes.
Hence the very different presentation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would still prefer having a consistent format for everything (i.e. always have a list for the list of parameters), to make it easy for the reader to reference information.

I'm having an idea that in the preface, we can specify the format we're using to signify what should be typed into the command and what should be replaced by the reader's own data. Since the distinction matters a lot and we've actually taken it for granted thus far. e.g. We'll say "All parts of the command that are typed in CAPITAL_LETTERS_WITH_UNDERSCORES are parameter data to be filled in by your particular circumstance."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I have revised this.

[big red]#List of Parameters#:

The old and new passwords of the application. +
Note that you only need to use spaces to seperate the two passwords. There is no prefix for this command!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be in a "Note" block

@@ -372,21 +451,35 @@ Examples:
* `setemail default \new_email@gmail.com`
* `setemail \old_email@gmail.com \new_email@gmail.com`

[big red]#List of Parameters#:

The old email that you have set to the current one that you would want to use. +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comments as for setpass command

Copy link
Collaborator

@OrangeJuice7 OrangeJuice7 left a comment

Choose a reason for hiding this comment

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

Left some comments for you to review.

I would like to give an overall suggestion for the User Guide: Put the List of Parameters section right under the Format section, rather than at the end of the command section. This makes them easier to reference.

@prokarius prokarius merged commit d62077e into master Nov 11, 2018
@prokarius prokarius deleted the add-tips-in-ug branch November 11, 2018 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation for repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants