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

Rewrite documentation #72

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dsonck92
Copy link
Contributor

Rewrite the documentation to reflect the recent changes and design decisions.

Closes: #63

- Mention the possibility of future
  return values
- Unify the documentation format of functions:
  - Function name in "code" font
  - Short introduction without references to
    parameter names
  - List of parameters with type and mandatory
    status, followed by a description
  - Return format as JSON (but tagged as yaml
    in MD file for better highlighting)
    followed by a description of the fields

Signed-off-by: Daniel Sonck <[email protected]>
@dsonck92
Copy link
Contributor Author

@tasleson I've done half of it as you might want to check whether my formatting changes are acceptable. I rather know ahead of time instead of doing it all and then do it all again. You can tell that I stopped at "Initiator operations" and yes, I will add the return values to the nfs as mentioned in #63.

@tasleson
Copy link
Member

@dsonck92 Wow, all I was thinking was just update the existing to add the return value for nfs and document that dictionaries returned may have items added to them over time. I think the changes look good, although I am wondering if it would be better to not show the domain of values like [block, fs] as one could conclude that the value for the key type is an array instead of one of the possible as it shows json?

@dsonck92
Copy link
Contributor Author

@tasleson That's a good question, I was a bit undecided on this point. I write the type of the field as its value (string, number) but for type I did want to convey that it may return exactly one of two options. However in the domain of the language I was rather limited, though I could also just write it as a non-styled code block and do something like "block"|"fs" indicating both it being a string but also limited to these values.

Yes I originally started out with the extra lines but while scrolling I discovered multiple formats, one of which that wasn't properly written to show properly after markdown. So after fixing this (seemingly old) format, I figured I could try to apply it to the rest.

Yes, the other option is to just say "string" and let people read the description in the accompanying text, which already mentions the 2 cases and what can be done with them.

@tasleson
Copy link
Member

I noticed you're using yaml for the markdown, but it's actually json. Would something like this work? I'm also wondering if we should include the descriptive text too, "returns an array of objects, each object consists of ..." in addition to the actual example?

[
   {
      "name":       string,
      "size":       number,
      "free_size":  number,
      "uuid":       string,
      "type":       string"domain[block|fs]",
   },
]

@dsonck92
Copy link
Contributor Author

dsonck92 commented Sep 14, 2020

Yes the primary reason for using YAML is that it has a bit more freedom to use it to color the syntax while still conveying some meaning. Mainly as raw values like string are invalid json, which I assume causes the red background in your example. Perhaps no styling is clearer, or we literally put an example and comment:

[
  {
    "name": "vol-1", // The name of the pool
    "size": 2048, // The size of the pool
    // ....
    "type": "block", // Type of the pool, possible values: "block", "fs"
  }
]

Using Javascript to enable the comments but still being essentially equal to JSON

@tasleson
Copy link
Member

@dsonck92 I like the Javascript approach!

@dsonck92
Copy link
Contributor Author

Alright, then I will apply that to the rest then. I will see how far I can get this evening with the documentation.

@dsonck92
Copy link
Contributor Author

dsonck92 commented Nov 5, 2020

@tasleson just letting you know that I'm likely going to continue on this in the weekend of 15 Nov (maybe upcoming weekend) as I would finally have more time to work on my own things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand API documentation for nfs_export_add
2 participants