-
Notifications
You must be signed in to change notification settings - Fork 350
Feature/fastify examples hello world #25
base: main
Are you sure you want to change the base?
Conversation
A very basic example of a base route for the fastify framework. More verbose descriptions are given as hello world examples are often read by folks new to eco system.
basic hello world in fastify
markdown lint corrections A markdownlintignore file has been added as there will be multiple folders which contain node module readmes that do not comply.
add contributor ghinks
|
||
```javascript | ||
fastify.get('/', async (request, reply) => { | ||
await setTimeoutPromise(2000).then(() => { |
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 then
is not needed.
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.
opps, I'm going to have to watch my promises
|
||
// Declare a route | ||
fastify.get('/', async (request, reply) => { | ||
await setTimeoutPromise(2000).then(() => { |
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 then
is not needed.
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.
opps
return { hello: 'world' } | ||
}) | ||
|
||
fastify.listen(APP_PORT, () => fastify.log.info(`server listening on ${fastify.server.address().port}`)) |
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 log line is not needed. listen
can error and it should be handled.
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.
Not sure about these suggested changes but for me it sounds better this way
Co-authored-by: Rodion Abdurakhimov <[email protected]>
quickly fix embarrassing promise abuse in code and README.md
change language as suggested by @rodion-arr
try { | ||
// if no callback is give a promise is returned | ||
await fastify.listen(3000) | ||
fastify.log.info(`server listening on ${fastify.server.address().port}`) |
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.
fastify.log.info(`server listening on ${fastify.server.address().port}`) |
This log line should not be needed, this info is logged by fastify itself now
try { | ||
// if no callback is give a promise is returned | ||
await fastify.listen(3000) | ||
fastify.log.info(`server listening on ${fastify.server.address().port}`) |
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.
fastify.log.info(`server listening on ${fastify.server.address().port}`) |
This log line should not be needed, this info is logged by fastify itself now
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.
lgtm with a nit
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.
So a couple notes: the REAME shouldn't be in the vein of a tutorial - it should be more like a README if you were to publish this project as a standalone project on GitHub. Here are a few generic examples of the style I'm talking about.
If you'd like, I'd definitely recommend posting the README as a blog post that points to the code that exists within this repository. Putting some kind of note at the beginning like "This is a guide for the Fastify "Hello, world!" example in nodejs/examples, which you can find [here] (link)!"
Overall, this looks good for a hello world, though the intent of this repo is definitely to go beyond hello world. I'm happy to merge this on the assumption that we'll get some examples that go beyond it, though if we don't I want to state that we may consider temporarily removing it so we don't have content that could be potentially frustrating to someone landing here doesn't align with the stated mission of the repo (I think it does fit in the context of "beyond hello world" since "hello world" is kinda key to going beyond "hello world").
Once that's addressed + @mcollina's requests are addressed I'd be happy to merge this.
A hello world fastify server example
( more advanced to follow )