Skip to content

RLPNC-7516: Add Score_if_null to Java Bindings for record-similarity#233

Merged
iredpath merged 6 commits intomasterfrom
RLPNC-7516-master-score-if-null-in-record-sim
Apr 30, 2024
Merged

RLPNC-7516: Add Score_if_null to Java Bindings for record-similarity#233
iredpath merged 6 commits intomasterfrom
RLPNC-7516-master-score-if-null-in-record-sim

Conversation

@fhasanaj
Copy link
Copy Markdown
Contributor

No description provided.

@ethteck
Copy link
Copy Markdown
Contributor

ethteck commented Apr 23, 2024

Review is all good on the names side

@ethteck
Copy link
Copy Markdown
Contributor

ethteck commented Apr 25, 2024

@seth-mg @asoosprecognox aside from the merge conflict, does this look alright to you? If I should reach out to someone else to get this reviewed, let me know

@asoosprecognox
Copy link
Copy Markdown
Contributor

Maybe add a scoreIfNull in the test that is a value and not just null, if that makes sense with the business logic. Otherwise it looks good if sonar is happy (no major/critical issues) after the merge conflicts are resolved

# Conflicts:
#	json/src/test/java/com/basistech/rosette/apimodel/RecordSimilarityRequestTest.java
@iredpath
Copy link
Copy Markdown
Contributor

@fhasanaj there's another merge conflict now after the RLPNC-7512 merge

# Conflicts:
#	json/src/test/java/com/basistech/rosette/apimodel/recordsimilarity/RecordSimilarityResponseTest.java
@iredpath
Copy link
Copy Markdown
Contributor

LGTM, @asoosprecognox or @seth-mg it would be great if one of you could take one more look before we merge this in

@fhasanaj
Copy link
Copy Markdown
Contributor Author

@asoosprecognox or @seth-mg can we get an approval from one of you please

Copy link
Copy Markdown
Contributor

@asoosprecognox asoosprecognox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iredpath iredpath merged commit 266cdee into master Apr 30, 2024
@iredpath iredpath deleted the RLPNC-7516-master-score-if-null-in-record-sim branch April 30, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants