Introduce Scanner.ChangeSetListener #12769
Conversation
+ Update testing for all Listeners
There was a problem hiding this comment.
There is a chance to cleanup this class, in particular by consolidating the "warn" logs.
They should be INFO or DEBUG level, not WARN, since we just log and ignore the failure, and the Scanner continues to operate as nothing happened.
Furthermore, remove the Scanner.warn() method, since in the rest of the class LOG.warn() is used instead.
I would prefer to see LOG.info()/debug() everywhere.
interface Listener should extend java.util.EventListener.
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java
Outdated
Show resolved
Hide resolved
|
@joakime to expedite my feedback (as my time is short).. I'm working on a branch that implements my suggestions |
| @@ -572,7 +614,7 @@ public void setReportExistingFilesOnStartup(boolean reportExisting) | |||
| _reportExisting = reportExisting; | |||
| } | |||
|
|
|||
| public boolean getReportExistingFilesOnStartup() | |||
| public boolean isReportExistingFilesOnStartup() | |||
There was a problem hiding this comment.
I guess if this method isn't referenced anywhere in jetty we might be able to get away with this method rename, but technically we should do a deprecation.
There was a problem hiding this comment.
This was changed to follow Jetty format / syntax.
Also, internally, the Scanner uses this method now, instead of the raw field.
This allows for folks extending Scanner have a better experience, as this now follows Java recommendations.
This is a small subset of the changes from PR #12583 made as a new PR just so we can track these changes separately from the deploy changes.