Skip to content
This repository was archived by the owner on Dec 31, 2025. It is now read-only.

Remove slf4j dependency#60

Merged
tylertreat merged 1 commit into
masterfrom
remove-logging
Jun 6, 2017
Merged

Remove slf4j dependency#60
tylertreat merged 1 commit into
masterfrom
remove-logging

Conversation

@ColinSullivan1

Copy link
Copy Markdown
Member

This PR is basic housekeeping to keep the client libraries trim - we do not need to log. Throwing exceptions (and in rare cases writing to stdout/stderr) should suffice.

@coveralls

coveralls commented Jun 2, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 92.857% when pulling 0153b3e on remove-logging into f8782a0 on master.

@tylertreat tylertreat left a comment

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.

lgtm

logger.debug("stan: publish interrupted");
logger.debug("Full stack trace:", e);
// Thread.currentThread().interrupt();
// TODO: ignore for now, but re-evaluatate this

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.

Do we have a way of tracking these TODOs? GitHub issues?

@petemiron petemiron left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. To Tyler's point, we need to have a strategy on the TODO Exception Handling. I think we need to throw Exceptions, but agree we can't break existing method calls and currently not worse than just logging in debug.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants