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.

Sign up for GitHub

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

Jump to bottom

Make the relationship between the Reporter and Closeable interfaces explicit. #1307

Merged
merged 3 commits into from
May 1, 2018
Merged

Make the relationship between the Reporter and Closeable interfaces explicit. #1307

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fd8a18
Resolves #1305 and reimplements #743. Credit to @obfischer for the in…
tjcelaya Apr 27, 2018
8a2b495
Fix Code Climate check and call out the java.io.Closeable interface i…
tjcelaya Apr 27, 2018
5a17238
Incorporate feedback about explicit implements in order to maintain b…
tjcelaya Apr 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion metrics-core/src/main/java/com/codahale/metrics/Reporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.codahale.metrics;

import java.io.Closeable;

/*
* A tag interface to indicate that a class is a Reporter.
*/
public interface Reporter {
public interface Reporter extends Closeable {

}
5 changes: 3 additions & 2 deletions metrics-core/src/main/java/com/codahale/metrics/ScheduledReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.Closeable;
import java.util.Collections;
import java.util.Locale;
import java.util.Set;
Expand All @@ -19,11 +18,13 @@
* The abstract base class for all scheduled reporters (i.e., reporters which process a registry's
* metrics periodically).
*
* Since {@link Reporter} extends {@link java.io.Closeable} this class also satisfies that interface.
*
* @see ConsoleReporter
* @see CsvReporter
* @see Slf4jReporter
*/
public abstract class ScheduledReporter implements Closeable, Reporter {
public abstract class ScheduledReporter implements Reporter {
Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove Closeable from ScheduledReporter, because the change is binary incompatible. That means we will force all 3rd reporters to upgrade to the latest version of metrics-core. I think it should be fine to just add Closeable to Reporter. It should be backward-compatible because ScheduledReporter and JmxReporter implement it already.


private static final Logger LOG = LoggerFactory.getLogger(ScheduledReporter.class);

Expand Down
4 changes: 2 additions & 2 deletions metrics-jmx/src/main/java/com/codahale/metrics/jmx/JmxReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import javax.management.MBeanServer;
import javax.management.ObjectInstance;
import javax.management.ObjectName;
import java.io.Closeable;
import java.lang.management.ManagementFactory;
import java.util.Collections;
import java.util.Locale;
Expand All @@ -30,8 +29,9 @@

/**
* A reporter which listens for new metrics and exposes them as namespaced MBeans.
* Since {@link Reporter} extends {@link java.io.Closeable} this class also satisfies that interface.
*/
public class JmxReporter implements Reporter, Closeable {
public class JmxReporter implements Reporter {
Copy link
Member

Choose a reason for hiding this comment

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

The same comment as for ScheduledReporter

/**
* Returns a new {@link Builder} for {@link JmxReporter}.
*
Expand Down