Skip to content
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

Added request logging #270

Closed
wants to merge 3 commits into from
Closed

Added request logging #270

wants to merge 3 commits into from

Conversation

stofolus
Copy link

@stofolus stofolus commented Sep 29, 2017

This is an implementation for #231

@request-info
Copy link

request-info bot commented Sep 29, 2017

We would appreciate it if you could provide us with more info about this issue/pr!

@welcome
Copy link

welcome bot commented Sep 29, 2017

Thanks for opening this pull request! Please check out our contributing guidelines.

@bkeepers
Copy link
Contributor

At first glance this is looking good. I'll pull it down and test it out soon. Thanks for the PR @stofolus!

@bkeepers
Copy link
Contributor

Ignoring this for now since the plan is to move to jest: #288

@bkeepers bkeepers closed this Oct 15, 2017
@bkeepers
Copy link
Contributor

Sorry, this was from an overzealous copy/paste on tabs that I had open.

@bkeepers bkeepers reopened this Oct 15, 2017
Copy link
Member

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, there's a syntax note that should be addressed.

lib/server.js Outdated
path: req.path,
ip: req.ip,
hostname:
req.hostname,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be on one line, would you mind making that fix please?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I will take care of this later today

@bkeepers
Copy link
Contributor

@stofolus thanks again for getting this started. After playing around with it a little, I decided to spend a little more time optimizing the logging setup. #322 is the result of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants