Skip to content

Conversation

miladsade96
Copy link

Changes:

  1. Correcting state management bug
  2. Handling null value properly

Copy link

google-cla bot commented Sep 18, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@miladsade96
Copy link
Author

miladsade96 commented Sep 18, 2025

I have signed the google cla, but still shows me as unsigned. Is there any way to force cla/google to reload or recheck the cla status?

@cpovirk
Copy link
Member

cpovirk commented Sep 18, 2025

I clicked the link for rescanning it from https://github.com/google/guava/pull/8006/checks?check_run_id=50725139081, and now it's showing as passing.

If this PR fixes a bug, please add a test that fails without the PR and passes with it.

@miladsade96
Copy link
Author

@cpovirk
I refactored the code but the behavior is the same as before.
Is it necessary to add more tests to the test class?

@cpovirk
Copy link
Member

cpovirk commented Sep 19, 2025

Ah, OK, I was going off the PR description.

It looks to me like the PR introduces a bug: It no longer allows computeNext() to return a null to be returned to users. Instead, it treats any null as the end of the iteration. (The docs note: "This class supports iterators that include null elements.".) From what I've seen so far, I don't think we have tests for that case. We probably should.

@chaoren
Copy link
Member

chaoren commented Sep 19, 2025

critical state management bug

Could you please describe what the bug is in the commit message and PR description?

Handling null value properly

Please describe where in the existing code was null not properly handled.

I refactored the code but the behavior is the same as before.

If the behavior is the same as before, then where is the bug?

There's a lot of shuffling around of existing code in the commit and it's not properly formatted, so it's difficult to tell what actually changed from just the diff.

@chaoren chaoren added type=defect Bug, not working as expected package=base status=triaged P3 no SLO labels Sep 19, 2025
@cpovirk cpovirk closed this Sep 24, 2025
@miladsade96 miladsade96 deleted the DEV_minorFixes branch September 26, 2025 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 no SLO package=base status=triaged type=defect Bug, not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants