Updated select samples to use async/await. #1012
Conversation
googlebot
added
the
cla: yes
MylesBorins
left a comment
MylesBorins
left a comment
There was a problem hiding this comment.
LGTM
Some nits inline about when to use an async function and about some of the logic that moved around
appengine/analytics/app.js
Outdated
| const GA_TRACKING_ID = process.env.GA_TRACKING_ID; | ||
|
|
||
| function trackEvent(category, action, label, value) { | ||
| async function trackEvent(category, action, label, value) { |
There was a problem hiding this comment.
if we are not using the await keyword inside the function I don't believe we need to convert it to an async function. We should still be able to await the function if it returns a thenable (e.g. promise)
| .order('timestamp', {descending: true}) | ||
| .limit(10); | ||
|
|
||
| return datastore.runQuery(query).then(results => { |
There was a problem hiding this comment.
is this logic moved somewhere else?
There was a problem hiding this comment.
Yeah, it's moved to the "/" hander function (~Line 73):
function getVisits() {..}
app.get('/', async (req, res, next) => {
...
let results = await getVisits();
let entities = results[0];
let visits = entities.map(
entity => `Time: ${entity.timestamp}, AddrHash: ${entity.userIp}`
);
...
appengine/datastore/app.js
Outdated
| .catch(next); | ||
| try { | ||
| await insertVisit(visit); | ||
| let results = await getVisits(); |
There was a problem hiding this comment.
is there a reason to use let instead of const here?
ace-n
left a comment
ace-n
left a comment
There was a problem hiding this comment.
LGTM once my + @MylesBorins comments are addressed.
appengine/datastore/app.js
Outdated
| * @param {object} visit The visit record to insert. | ||
| */ | ||
| function insertVisit(visit) { | ||
| async function insertVisit(visit) { |
There was a problem hiding this comment.
Ditto to @MylesBorins above comment - do we need this async?
appengine/datastore/app.js
Outdated
| * Retrieve the latest 10 visit records from the database. | ||
| */ | ||
| function getVisits() { | ||
| async function getVisits() { |
| 'http://metadata.google.internal/computeMetadata/v1/project/project-id'; | ||
|
|
||
| function getProjectId() { | ||
| async function getProjectId() { |
appengine/pubsub/app.standard.yaml
Outdated
| env_variables: | ||
| PUBSUB_TOPIC: YOUR_TOPIC_NAME | ||
| #PUBSUB_TOPIC: YOUR_TOPIC_NAME | ||
| PUBSUB_TOPIC: sample-topic |
There was a problem hiding this comment.
Did/will you update the docs accordingly, if necessary?
4 participants
As requested by Steren.