Skip to content

Make logger configurable and log socket data.#67

Merged
dblock merged 1 commit intomasterfrom
log-socket-data
Mar 4, 2016
Merged

Make logger configurable and log socket data.#67
dblock merged 1 commit intomasterfrom
log-socket-data

Conversation

@dblock
Copy link
Copy Markdown
Collaborator

@dblock dblock commented Feb 29, 2016

@dblock dblock mentioned this pull request Feb 29, 2016
@dblock dblock force-pushed the log-socket-data branch from d6a5f56 to c324f20 Compare March 3, 2016 20:00
@dblock
Copy link
Copy Markdown
Collaborator Author

dblock commented Mar 3, 2016

@mikz FYI I finished what you started in #50 here.

Comment thread lib/slack/real_time/client.rb Outdated

def send_json(data)
fail ClientNotStartedError unless started?
logger.debug("#{self.class.name}#send_data") { data }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be #send_json ? maybe using "#{self.class}##{__method__}" would make it automatic.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

👍

Generally I wonder about logging data here, it's a binary blob most of the time. Thoughts?

@mikz
Copy link
Copy Markdown
Contributor

mikz commented Mar 4, 2016

This looks great! Good job 👍

@dblock dblock force-pushed the log-socket-data branch 2 times, most recently from c152765 to a794bfd Compare March 4, 2016 13:24
dblock added a commit that referenced this pull request Mar 4, 2016
Make logger configurable and log socket data.
@dblock dblock merged commit 472d508 into master Mar 4, 2016
@dblock dblock deleted the log-socket-data branch March 4, 2016 14:56
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