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
Add ESLint #16
Add ESLint #16
Conversation
@chrishelgert Can you please review my changes? |
Sure, will look later @ it :) |
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.
@danielbayerlein please check the review-comments ;) the rest looks fine
@@ -7,7 +7,8 @@ describe('helpers', () => { | |||
const spans = defaultConfig.spans; | |||
|
|||
beforeEach(() => { | |||
spans.forEach((span) => { | |||
spans.forEach((currentSpan) => { |
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.
should have a different meaning - maybe make spans a let and use map to iterate over the spans and clear the responses:
let spans = defaultConfig.spans;
// ....
spans = spans.map((currentSpan) => {
const span = currentSpan;
span.responses = [];
return span;
});
// ...
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.
Done
|
||
// Add the "/return-status/{statusCode}" route | ||
server.route({ | ||
method: 'GET', | ||
path: '/return-status/{statusCode}', | ||
handler: function (request, reply) { | ||
handler: function handler(request, reply) { |
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.
why no arrow-function and parse the statusCode initial?
handler: (request, reply) => {
const statusCode = parseInt(request.params.statusCode, 10);
return reply(statusCode).code(statusCode);
};
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.
Done
@@ -16,7 +14,8 @@ module.exports = (server, spans) => { | |||
}); | |||
}); | |||
|
|||
spans.forEach((span) => { | |||
spans.forEach((currentSpan) => { |
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.
same as above with map or?
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.
spans
is not reused. I think it's 👌
@@ -5,50 +5,50 @@ const onHeadersListener = require('./helpers/on-headers-listener'); | |||
const socketIoInit = require('./helpers/socket-io-init'); | |||
|
|||
// hapi.js plugin register function | |||
var middlewareWrapper = function (server, options, next) { | |||
options = validate(options); | |||
const middlewareWrapper = function middlewareWrapper(server, options, next) { |
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.
why no arrow-function?
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.
Done
@chrishelgert Thanks for the review 👍 |
See #13