Started with async/await/promise and hapi v17 migration #68
Conversation
paazmaya
changed the title
|
Anyone interested? Maybe @hollsk since this person seems to be the one making most latest merges.... |
|
@hollsk this PR is again up to date against |
There was a problem hiding this comment.
Absolutely fantastic work @paazmaya. This is really great!
One comment on the PR as a whole is that it introduces async functions and uses await nicely. However the models (mainly) still rely on directly using promises and returning them, as well as handling errors within them. IMO it would be best to adopt a singular style across the codebase. Which for me would be async/await. Though it would be good to get others opinions on this.
There may also be a broader discussion about using a logging library that can handle things like logging debug messages in development etc.
| return reply().code(500); | ||
| handler: async function(request, reply) { | ||
|
|
||
| const task = await model.task.getById(request.params.id); |
There was a problem hiding this comment.
Do we want to handle the error logging in our model, or here in the route?
There was a problem hiding this comment.
No idea. Perhaps I could work on those in a separate PR...
There was a problem hiding this comment.
I think that would make sense. We generally want to tackle logging a bit within the web-services. Theres a number of .code(500) responses without any further information for the client.
|
@joeyciechanowicz there might be issues still from the merge yesterday, since there were plenty of conflicts, hence some lines might have re-apperred, such as the use of |
|
Now I remember, I did not want to touch them until MongoDB would be updated, and I did not want to do it in this PR, specially when I noticed the last of interest in early April... |
|
After this PR, there is still version cap in these two major modules:
|
|
@joeyciechanowicz how to proceed? |
Started with library upgrade which ended up getting more using async/await and so forth...