Improved opt-in logging support #7437
Conversation
|
/format |
|
I think you're overcomplicating things. The solution is really just to tweak the criteria when determining if a type is loggable or not. Currently, that criteria is set to the presence of an |
|
/format |
| @@ -59,6 +59,45 @@ public void update(DataLogger dataLogger, Example object) { | |||
| assertLoggerGenerates(source, expectedGeneratedSource); | |||
| } | |||
|
|
|||
| @Test | |||
| void basicOptInLogging() { | |||
There was a problem hiding this comment.
I'd call this something like "implicit" logging. The opt-in here is done on the field or method level, so the class itself is implicitly loggable
| @Logged double x; | ||
| @Logged double y; |
There was a problem hiding this comment.
A separate test that checks for a logged method would be useful, too
Cool; i didn't know that the current code worked directly with field + method elements |
|
/format |
changed getLoggedTypes() to be only called once
|
/format |
|
@Daniel1464 You'll need to merge the |
| @Logged public double getValue() { return 2.0; } | ||
| @Logged public boolean isActive() { return false; } |
There was a problem hiding this comment.
These would be better in a separate test, so we can check that just a logged field is enough to make a class logged, and that just a logged method is also enough. Having both in one test means it's ambiguous, and also means that if either field or method detection breaks (but not both), then it wouldn't be caught by the test cases.
@Test
void implicitLoggedFields() {
String source =
"""
package edu.wpi.first.epilogue;
class Example {
@Logged double x;
}
""";
// ...
}
@Test
void implicitLoggedMethods() {
String source =
"""
package edu.wpi.first.epilogue;
class Example {
@Logged
public double x() { return 42; }
}
""";
// ...
}Additionally, it'd be good to have another test to cover the null case of a class with no @Logged annotation and with no implicitly logged members to make sure we don't generate loggers for things that users didn't opt into
|
/format |
Allows users to annotate fields and methods with @Logged without putting the @Logged(strategy = Strategy.OPT_IN) moniker on their class to allow opt-in logging on those fields/methods. Solves #6916.