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

Cleaned up memcached example and added tests. #26

Merged
merged 1 commit into from
Nov 13, 2015
Merged

Conversation

jmdobry
Copy link
Member

@jmdobry jmdobry commented Nov 13, 2015

No description provided.

@@ -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]

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.

@JustinBeckwith
Copy link
Contributor

LGTM.

@jmdobry
Copy link
Member Author

jmdobry commented Nov 13, 2015

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']

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. :)

Copy link
Member Author

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?

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

Copy link
Member Author

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.

@theacodes
Copy link

LGTM w/ final nit.

jmdobry added a commit that referenced this pull request Nov 13, 2015
Cleaned up memcached example and added tests.
@jmdobry jmdobry merged commit 0475cde into master Nov 13, 2015
@jmdobry jmdobry deleted the fix-memcached branch November 13, 2015 22:54
telpirion pushed a commit that referenced this pull request Nov 9, 2022
gcf-merge-on-green bot pushed a commit that referenced this pull request Nov 9, 2022
…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)
grayside pushed a commit that referenced this pull request Nov 16, 2022
* chore: lock files maintenance

* chore: lock files maintenance
ace-n pushed a commit that referenced this pull request Nov 17, 2022
* chore: lock files maintenance

* chore: lock files maintenance
grayside pushed a commit that referenced this pull request Nov 17, 2022
* chore: lock files maintenance

* chore: lock files maintenance
ahrarmonsur pushed a commit that referenced this pull request Nov 17, 2022
* chore: lock files maintenance

* chore: lock files maintenance
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.

3 participants