-
Notifications
You must be signed in to change notification settings - Fork 49
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.
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
Refactoring of sequence testing code #299
Conversation
* | ||
* @param args What to do! | ||
* @param args This is the way |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What? I wasn't planning on anybody actually reading my comments... that's not fair! (But updated to something more sensical)
info("Updated state has last_config " + getTimestamp( | ||
((State) converted).system.last_config)); | ||
} else { | ||
warning("Unknown update type " + converted.getClass().getSimpleName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this severe enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
warning("Not resetting config b/c trace is enabled."); | ||
recordMessages = false; | ||
if (debugLogLevel()) { | ||
warning("Not resetting config because debug is enabled."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't clear at first glance why you don't reset the config in this case. Can you add a comment to quickly spell it out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the warning message itself to clarify why
Should be functionality neutral -- no new tests or capabilities.