Skip to content

feat(app): improved socket.io logging #1236

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

Merged
merged 1 commit into from
Sep 2, 2015

Conversation

kingcody
Copy link
Member

@kingcody kingcody commented Sep 1, 2015

No description provided.

@@ -14,7 +14,7 @@ function onDisconnect(socket) {
function onConnect(socket) {
// When the client emits 'info', this listens and executes
socket.on('info', function(data) {
console.info('[%s] %s', socket.address, JSON.stringify(data, null, 2));
console.info(`SocketIO ${socket.nsp.name} [${socket.address}] ${JSON.stringify(data, null, 2)}`);
Copy link
Member Author

Choose a reason for hiding this comment

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

This line needs to be <=100.

Copy link
Member

Choose a reason for hiding this comment

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

characters long?

@kingcody
Copy link
Member Author

kingcody commented Sep 1, 2015

@Awk34 what do you think about this change? (sans jscs failure)

@Awk34
Copy link
Member

Awk34 commented Sep 1, 2015

LGTM

@kingcody
Copy link
Member Author

kingcody commented Sep 1, 2015

Sounds good, I'll fix the line length.

@kingcody kingcody force-pushed the feature/improve-socketio-logging branch from e661950 to c1fd93f Compare September 1, 2015 19:06
@kingcody kingcody closed this Sep 1, 2015
@kingcody kingcody reopened this Sep 1, 2015
@kingcody
Copy link
Member Author

kingcody commented Sep 1, 2015

@Awk34 check it again, I added a socket.log method.

Awk34 added a commit that referenced this pull request Sep 2, 2015
@Awk34 Awk34 merged commit 42c8fa0 into canary Sep 2, 2015
@kingcody kingcody deleted the feature/improve-socketio-logging branch September 2, 2015 22:50
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.

2 participants