-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
NIFI-12480: Updated MapRecord's toString() method to use the Serializ… #8132
Conversation
…edForm of the record when available and fixed bugs around ensuring that the serialized form is properly set
if (serializedForm.isEmpty()) { | ||
return Optional.empty(); | ||
} | ||
|
||
if (isSerializedFormReset()) { | ||
return Optional.empty(); | ||
} |
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.
if (serializedForm.isEmpty()) { | |
return Optional.empty(); | |
} | |
if (isSerializedFormReset()) { | |
return Optional.empty(); | |
} | |
if (serializedForm.isEmpty() || isSerializedFormReset()) { | |
return Optional.empty(); | |
} |
return switch (fieldType) { | ||
case ARRAY, RECORD, MAP, CHOICE -> false; | ||
default -> true; | ||
}; |
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.
This switch statement style is from later versions of Java, is the fix in this PR not going to be backported at all?
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.
We absolutely should adopt language niceties as we go forward and hopefully backport considerations don't cause us to avoid that. Backports are fair game where it makes sense but should happen with specific PRs and this will be increasingly true as we go forward.
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.
No, I have no plans to backport.
return; | ||
if (serialized instanceof final String serializedString) { | ||
final boolean serializedPretty = serializedString.contains("\n"); | ||
if (serializedPretty == this.prettyPrint) { |
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 not common for boolean logic
if (serializedPretty == this.prettyPrint) { | |
if (serializedPretty && this.prettyPrint) { |
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.
These are not equivalent. I'm not checking if they are both true - I am checking if they are equal.
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.
…edForm of the record when available and fixed bugs around ensuring that the serialized form is properly set
Summary
NIFI-00000
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000
NIFI-00000
Pull Request Formatting
main
branchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-check
Licensing
LICENSE
andNOTICE
filesDocumentation