fix several examples #3310
fix several examples #3310
Conversation
**1. examples/markdown** http://localhost:3000/fail Error: Failed to lookup view "missing" in views directory **2. examples/params** http://localhost:3000/users/0-2 users tj, tobi <-- lost loki Because user/0 is tj, that’s means the 0 is the data, not index of array. So, users/0-2 must be include user/2. **3. examples/route-separation** http://localhost:3000 Click link "posts", jumped to the "users" page. **4. examples/view-constructor** http://localhost:3000/ ReferenceError: https is not defined **5. examples/online, examples/search, and examples/session** // first (if you have not done it yes): // $ npm install redis // $ brew install redis <-- lost this line // $ redis-server If the user has installed redis, then there is no problem reinstalling redis. But if here does not prompt the user to install redis, this example will run failure, the user will be confused.
examples/online/index.js
Outdated
| @@ -1,7 +1,11 @@ | |||
| // first: | |||
| // $ npm install redis online | |||
| // first (if you have not done it yes): | |||
examples/online/index.js
Outdated
| // $ npm install redis online | ||
| // first (if you have not done it yet): | ||
| // $ npm install redis | ||
| // $ brew install redis |
There was a problem hiding this comment.
If install instructions are added, can they be more accessible to the general user base? For example, this only works if the user is both (a) using MacOS and (b) has Homebrew installed (not of which is noted in here, just like how installing Redis was not noted before). Perhaps remove the command altogether and just add a link to Redis's site for how to install?
There was a problem hiding this comment.
I agree with you, Doug.
examples/search/index.js
Outdated
| // $ npm install redis | ||
| // $ brew install redis |
There was a problem hiding this comment.
Same Redis install instructions issue.
examples/session/index.js
Outdated
| // $ npm install redis | ||
| // $ brew install redis |
There was a problem hiding this comment.
Same Redis install instructions issue.
examples/online/index.js
Outdated
| // $ npm install redis online | ||
| // first (if you have not done it yet): | ||
| // $ npm install redis | ||
| // $ brew install redis |
There was a problem hiding this comment.
Not sure I agree with adding this. In an example the point is to show express, the assumption being you already know you need a redis instance running somewhere. I guess I might be fine with saying ensure you have redis running on its default port, since it does not assume osx and brew.
There was a problem hiding this comment.
Oh sorry @dougwilson your messages came in after I added mine :)
|
Thanks very much! I split your four changes into four commits to land, and also updated the test we had for your |
|
Thanks Doug. |
1. examples/params
http://localhost:3000/users/0-2
users tj, tobi <-- lost loki
Because user/0 is tj, that’s means the 0 is the data, not index of array.
So, users/0-2 must be include user/2.
2. examples/route-separation
http://localhost:3000
Click link "posts", jumped to the "users" page.
3. examples/view-constructor
http://localhost:3000/
ReferenceError: https is not defined
4. examples/online, examples/search, and examples/session
// install redis first:
// https://redis.io/
// then:
// $ npm install redis
// $ redis-server
If the user has installed redis, then there is no problem reinstalling redis. But if the user has not done it yet, and here does not prompt the user to install redis, this example will run failure, the user will be confused.