-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Cleaned up memcached example and added tests. #26
Conversation
@@ -13,6 +13,7 @@ and applications on [Google App Engine](http://cloud.google.com/nodejs). | |||
### Frameworks | |||
|
|||
- Express.js - [Source code][express_1] | [App Engine Tutorial][express_2] | [Live demo][express_3] | [Documentation][express_4] | |||
- Express.js + Memcached Sessions - [Source code][express_5] | [Documentation][express_6] |
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 that you don't have to fix now: it would be better to have more described anchors here. express_6
is kinda of funny.
LGTM. |
c63cad4
to
656d99a
Compare
I simplified the test and the readme. Should be good to go. |
key: 'view:count', | ||
proxy: 'true', | ||
store: new MemcachedStore({ | ||
hosts: [process.env.MEMCACHE_URL || '127.0.0.1:11211'] |
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.
Now that you slimmed down the readme, a little comment here about how MEMCACHE_URL
is set by GAE would be helpful. :)
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.
It's set by GAE? So it doesn't need to be set in the app.yaml
file?
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.
Nvm, just saw the full thing. A comment in the readme about the memcache
host then. :P
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 think there is one at the bottom.
LGTM w/ final nit. |
656d99a
to
5784567
Compare
5784567
to
4b16030
Compare
Cleaned up memcached example and added tests.
🤖 I have created a release \*beep\* \*boop\* --- ## [1.2.0](https://www.github.com/googleapis/nodejs-contact-center-insights/compare/v1.1.0...v1.2.0) (2021-08-11) ### Features * add new issue model API methods ([b73f2e9](https://www.github.com/googleapis/nodejs-contact-center-insights/commit/b73f2e93ddb572519685190643c1f02174813658)) * support Dialogflow and user-specified participant IDs ([b73f2e9](https://www.github.com/googleapis/nodejs-contact-center-insights/commit/b73f2e93ddb572519685190643c1f02174813658)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
…nter-insights (#2817) - feat: initial generation of templated files - feat: add initial samples (#2) - chore(deps): update dependency mocha to v9 (#3) - chore: release 1.0.0 (#1) - chore: release 1.0.1 (#12) - chore: release 1.0.2 (#18) - chore: release 1.1.0 (#21) - chore: release 1.2.0 (#26) - chore: release 1.2.1 (#29) - chore: release 1.3.0 (#36) - chore: release 1.4.0 (#40) - samples: get operation (#52) - samples: create conversation, create analysis, export data to BigQuery (#34) - samples: enable pubsub notifications (#48) - samples: modify TTL in create conversation with TTL sample (#57) - samples: create topic model (#49) - chore: release 1.5.0 (#58) - samples: set project-level TTL (#51) - samples: create custom highlights (#53) - chore: release 1.6.0 (#65) - chore: release 1.7.0 (#69) - chore: release 1.8.0 (#75) - chore: release 1.9.0 (#81) - chore(main): release 1.11.0 (#107) - build: add retries for flaky test (#109) - build!: update library to use Node 12 (#122) - chore(main): release 2.0.0 (#124) - fix(deps): update dependency @google-cloud/bigquery to v6 (#127) - fix(deps): update dependency @google-cloud/pubsub to v3 (#126) - chore(main): release 2.0.1 (#129) - chore(main): release 2.1.0 (#134) - chore(main): release 2.1.1 (#139) - chore(deps): update dependency uuid to v9 (#142)
* chore: lock files maintenance * chore: lock files maintenance
* chore: lock files maintenance * chore: lock files maintenance
* chore: lock files maintenance * chore: lock files maintenance
* chore: lock files maintenance * chore: lock files maintenance
No description provided.