-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
docs: add basic docs for KeyValueRepository #1654
Conversation
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.
Great docs! Some minor nitpicks.
docs/site/Repositories.md
Outdated
### Define a KeyValue Datasource | ||
|
||
We first need to define a datasource to configure the key-value store. For | ||
better flexibility, we spilt the datasource definition into two files. |
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.
As a reader I'm left wondering where is the flexibility? Is this covered in this doc somewhere before this point? If not we may want to mention that the flexibility comes from being able to inject the config into the DataSource
class.
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.
typo: spilt change to split or splited ?
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.
Good catch. Will fix.
docs/site/Repositories.md
Outdated
### Define a KeyValueRepository | ||
|
||
The KeyValueRepository binds a model such as `ShoppingCart` to the | ||
`RedisDataSource`. The base `DefaultKeyValueRepository` provides an |
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.
base => case class
docs/site/Repositories.md
Outdated
testKV(); | ||
``` | ||
|
||
[keyvaluerepository]: |
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, this would explain the broken links above, this should be KeyValueRepository
.
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.
Argh, prettier
converts it to be lower case :-(.
docs/site/Repositories.md
Outdated
## Persisting Data without Juggler [Using MySQL database] | ||
## Access KeyValue Stores | ||
|
||
We can now access key-value stores such as [redis](Redis) using the |
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.
Missing link for redis.
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.
The content LGTM, could you please double-check that all links work correctly.
docs/site/Repositories.md
Outdated
### Perform Key Value Operations | ||
|
||
The KeyValueRepository provides a set of key based operations, such as `set`, | ||
`get`, `delete`, `expire`, `ttl`, and `keys`. See [KeyValueRepository] for a |
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.
See [KeyValueRepository] for a
Missing link?
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.
Fix this before landing this PR.
9c14ab4
to
fe4eca9
Compare
Comments addressed. PTAL. |
fe4eca9
to
7059eee
Compare
better flexibility, we spilt the datasource definition into two files. The json | ||
file captures the configuration properties and it can be possibly overridden by | ||
dependency injection. | ||
|
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.
sorry to chime in, but wouldn't be good to add the following NOTE?. Especially for new LB4 adopters?
NOTE:
The prefix redis is related to the name of the data source, so you can use any name you want here. Usually you use lb4 datasource name
to generate it, where name
is the prefix mentioned above.
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.
The effective name
is from https://github.com/strongloop/loopback4-example-shopping/blob/master/src/datasources/redis.datasource.json#L2 and the file name should not matter.
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 didn't notice the context of the example-shopping.
Add basic docs for
KeyValueRepository]
using loopbackio/loopback4-example-shopping#4 as an example.Checklist
npm test
passes on your machinepackages/cli
were updatedexamples/*
were updated