Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve logging #790

Merged
merged 8 commits into from
Jan 19, 2017
Merged

Conversation

SamCarlberg
Copy link
Member

The current logs are about 1100 lines of debugging stuff from guava and about zero lines of stuff from GRIP. This PR reverses that situation.

private static final Logger logger = Logger.getLogger(EventLogger.class.getName());

@Subscribe
public void eventPosted(LoggableEvent event) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to make it so that this event can be handled by multiple threads simultaneously?
If so you need to add that other annotation (not sure what its called)

Copy link
Member Author

Choose a reason for hiding this comment

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

@AllowConcurrentEvents I think

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not going to add the @AllowConcurrentEvents annotation; the event bus is synchronous so only one event will be posted at a time anyway.

@codecov-io
Copy link

codecov-io commented Jan 17, 2017

Current coverage is 52.51% (diff: 66.66%)

Merging #790 into master will increase coverage by 0.39%

@@             master       #790   diff @@
==========================================
  Files           238        240     +2   
  Lines          7668       7705    +37   
  Methods           0          0          
  Messages          0          0          
  Branches        739        742     +3   
==========================================
+ Hits           3996       4046    +50   
+ Misses         3488       3475    -13   
  Partials        184        184          

Sunburst

Powered by Codecov. Last update 2eb0506...9fae11f

.toString();
}

private static String simpleString(Socket<?> socket) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this method in the connection class instead of a helper method on the Socket object??
Throw this in a default method on the Socket interface.

Copy link
Member

@JLLeitschuh JLLeitschuh left a comment

Choose a reason for hiding this comment

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

Do the thing with @AllowConcurrentEvents and move the simple string method to the class it's creating a simple string on.

@SamCarlberg SamCarlberg merged commit 4ce1943 into WPIRoboticsProjects:master Jan 19, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants