-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add fix quality metadata and fixedFindings
for changeset entry
#32
Conversation
@@ -15,22 +16,33 @@ public final class CodeTFChangesetEntry { | |||
private final List<CodeTFChange> changes; | |||
|
|||
private final CodeTFAiMetadata ai; | |||
private final Strategy strategy; | |||
private final boolean provisional; |
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.
I think the spec already calls for this to be a boolean, so I'm probably too late on this feedback, but replacing "boolean" with "enum that has two items" leaves room for expansion and better describes what this is e.g.
enum Maturity { MATURE, PROVISIONAL; }
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.
Noted, but at this point I think we will need to revisit it at a later date.
this.strategy = strategy; | ||
this.provisional = provisional; | ||
this.fixedFindings = CodeTFValidator.toImmutableCopyOrEmptyOnNull(fixedFindings); |
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.
I assume all of these may be null
to allow for backwards compatibility, but do we expect to eventually make these required?
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.
I don't have a strong opinion about this. Provisional has a reasonable default of false
but strategy does not.
ad881f0
to
265ba8a
Compare
No description provided.