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

NBABasedBulkSearch::encode doesn't return on cancel #4793

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

mbien
Copy link
Member

@mbien mbien commented Oct 15, 2022

@BradWalker found in #4781 a minor bug where cancel.get() was checked but ignored.

The last check is redundant since it is the end of the method.

This might have been a copy and paste bug, since other methods have checks like if (cancel.get) return null;. I suppose it was meant to only remove the null but leave the return

@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) hints labels Oct 15, 2022
@mbien mbien added this to the NB16 milestone Oct 15, 2022
@mbien mbien requested a review from jlahoda October 15, 2022 18:58
@neilcsmith-net neilcsmith-net modified the milestones: NB16, NB17 Oct 19, 2022
Copy link
Member

@sdedic sdedic left a comment

Choose a reason for hiding this comment

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

I'd say the fix is OK; not sure if there are not other places where it is desirable to check for cancel flag.

@mbien
Copy link
Member Author

mbien commented Oct 25, 2022

@sdedic thanks for taking a look. I also thought that this was fairly low risk - most likely a (minor) copy&paste bug Brad discovered in his cleanup PR.

@mbien mbien merged commit 819c07a into apache:master Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hints Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants