Adding API to the docs #23
Conversation
Codecov Report
@@ Coverage Diff @@
## master #23 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 13 13
Lines 416 416
Branches 45 45
=====================================
Hits 416 416
Continue to review full report at Codecov.
|
fidian
left a comment
fidian
left a comment
There was a problem hiding this comment.
Rest of this looks good. Not sure if my single nod is good enough because there's a lot of words and I could easily have missed something.
|
|
||
| Usage | ||
| ----- | ||
| API |
There was a problem hiding this comment.
Should "reference" and "artifact" be children of the "API" heading?
If so, you need to demote the other headings. Lots of minor changes.
If not (and I recommend this), perhaps "API" could be changed to "Library Interface" or some other wording in order to narrow the scope to just the thing that you get from require()?
There was a problem hiding this comment.
ah. yeah I see what you are saying. Yes they should be demoted :(
README.md
Outdated
| }, (err) => { | ||
| console.log("Hit an error: " + err); | ||
| }); | ||
| ### `reference.initSearch([path])` |
There was a problem hiding this comment.
This one uses a dot. Others use a tilde. I think, if we are mirroring Node, we'd use a dot in all of the headings.
|
|
||
| git clone https://github.com/not-nexus/shelf-lib-js.git | ||
| cd shelf-lib-js | ||
| npm install |
There was a problem hiding this comment.
I know this wasn't changed, but it might be a good idea to add instructions for installing as a module to use? Like npm install shelf-lib --save.
README.md
Outdated
| // Import the library. | ||
| var shelfLib = require("shelf-lib")("<URL where pyshelf is hosted>"); | ||
|
|
||
| ### `shelfLib(origin, [libOptions])` |
There was a problem hiding this comment.
I think the comma before optional arguments is inside the square brackets. Like so: shelfLib(origin[, libOptions]). I wasn't sure about this, so I checked on Node.Js's documentation and that's how they have it.
README.md
Outdated
|
|
||
| Options | ||
| ----------- | ||
| * `origin` `{string}` - The protocol and host for connecting to Shelf. For example `https://api.shelf.com`. |
There was a problem hiding this comment.
I don't think you need the "For example" part, as you have an example below. You could specify that it must start with "https://" or "http://" though.
README.md
Outdated
|
|
||
| // Grab a reference to a bucket. | ||
| var reference = shelfLib.initReference("refName", "<super secret pyshelf API key>"); | ||
| * `refName` `{string}` - Could also be called "bucket", "storage" or "shelf". This represents a specific storage space. |
There was a problem hiding this comment.
I think it makes more sense to have the second sentence first. Then you're explaining what it is and then following that, you'd have extra information about it. I am not saying that this is an actual problem, I'm just saying it makes more sense to me.
README.md
Outdated
| // Grab a reference to a bucket. | ||
| var reference = shelfLib.initReference("refName", "<super secret pyshelf API key>"); | ||
| * `refName` `{string}` - Could also be called "bucket", "storage" or "shelf". This represents a specific storage space. | ||
| * `authToken` `{string}` - The authentication token used to authenticate with the `reference` |
There was a problem hiding this comment.
Needs a period at the end.
| }, (err) => { | ||
| console.log("Hit an error: " + err); | ||
| }); | ||
| * `path` `{string}` - Just the path to the artifact inside of the reference. It is `<path>` in `https://api.shelf.com/shadow/artifact/<path>`. |
There was a problem hiding this comment.
Again, I don't think you need an example here since there is one below. I also think you should remove the word "just", as it sounds informal. I don't think it really matters, but I think Tyler has called me out on it before.
There was a problem hiding this comment.
This one is much more important because it gives context to people who know how to use shelf.
README.md
Outdated
| Uploads the content provided to the path that the [artifact](#artifact) object to set up to point to. It returns a Promise which will resolve to have a path to where it was uploaded to. This should be usable with `reference~initArtifact`. | ||
|
|
||
| * `content` `{(string|Buffer|stream~ReadStream)}` - What you want uploaded as an artifact. | ||
| * Returns: `{Promise.<string>}`. |
There was a problem hiding this comment.
Even though you explain in the example before, it might be nice to explain what is being returned, along with the type.
README.md
Outdated
| var tagProperty1 = { | ||
| immutable: true, | ||
| value: "Tag-taggy" | ||
| The Second one is called `metadataValues` which is an object whose keys are the name of a property and the value is a `metadataProperty`. |
There was a problem hiding this comment.
I don't think "Second" should be capitalized here.
|
Sweet. Looks good. |
No description provided.