-
Notifications
You must be signed in to change notification settings - Fork 16
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
Bugfix/939 clob getall office filter #1024
base: develop
Are you sure you want to change the base?
Bugfix/939 clob getall office filter #1024
Conversation
Removed test method that had identical implemention.
…in current office and then all the other offices. Using vOffice and vClob to match naming conventions. Adding orderBy to the getClobsLike results for consistency.
} else { | ||
clobConsumer.accept(null); | ||
} | ||
clobConsumer.accept(clob); |
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.
are we sure null here isn't valid?
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 it is valid. Its just that when clob == null the variable clob already has null in it so clobConsumer.accept(clob) is the right line in either case. I didn't spot this myself, the IDE suggested it. More of a - 'Huh - its right. ok.'
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.
oh right we're just passing null anyways.
I'm redoing pr #937 to address #939.
develop had moved significantly since #937 had been started that it seemed cleaner to just grab the handful of changes.