Skip to content
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

Clean up AnyQueryNode code #13053

Merged
merged 4 commits into from Jan 31, 2024
Merged

Clean up AnyQueryNode code #13053

merged 4 commits into from Jan 31, 2024

Conversation

sabi0
Copy link
Contributor

@sabi0 sabi0 commented Jan 29, 2024

  • removed redundant field initializers
  • fixed a typo in the field name: minimumMatching_m_Elements
  • removed redundant instanceof checks
  • replaced type casts with a pattern variable
  • changed size() == 0 to isEmpty()
  • removed unnecessary explicit toString() calls

- removed redundant field initializers
- fixed a typo in the field name: `minimumMatching_m_Elements`
- removed redundant `instanceof` checks
- replaced type casts with a pattern variable
- changed `size() == 0` to `isEmpty()`
- removed unnecessary explicit `toString()` calls
@dweiss dweiss self-requested a review January 30, 2024 20:12
@dweiss dweiss self-assigned this Jan 30, 2024
@dweiss dweiss added this to the 9.10.0 milestone Jan 30, 2024
@dweiss
Copy link
Contributor

dweiss commented Jan 30, 2024

Thank you, LGTM. Could you add a changes entry as well?

if (clause instanceof FieldableNode) {
((FieldableNode) clause).setField(field);
}
if (clause instanceof FieldQueryNode qn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't backport to 9x, unfortunately. I'll just correct it manually when backporting. Or should we leave this on main only, @uschindler - wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could undo the pattern variable part, leaving it to #12295.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please.

Copy link
Contributor

@uschindler uschindler left a comment

Choose a reason for hiding this comment

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

Looks fine. Thanks for also fixing the typo in field name. 🤩

@dweiss dweiss merged commit 08e0f41 into apache:main Jan 31, 2024
4 checks passed
asfgit pushed a commit that referenced this pull request Jan 31, 2024
@sabi0 sabi0 deleted the AnyQueryNode branch January 31, 2024 16:21
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.

None yet

3 participants