Skip to content

Conversation

@anjalii-28
Copy link

Fixes #3220

@knative-prow knative-prow bot requested review from aslom and creydr December 13, 2025 22:24
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 13, 2025

CLA Missing ID CLA Not Signed

@knative-prow
Copy link

knative-prow bot commented Dec 13, 2025

Hi @anjalii-28. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@knative-prow knative-prow bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 13, 2025
@anjalii-28
Copy link
Author

This PR fixes the panic caused by DeletedFinalStateUnknown in the ConfigMap informer
and adds tests to ensure delete events are handled safely.

PTAL, thanks!

@dprotaso
Copy link
Member

/ok-to-test

@knative-prow knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 16, 2025
@dprotaso
Copy link
Member

You'll need to sign the CLA - see this comment #3301 (comment)

@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.62%. Comparing base (e853b1d) to head (e0d3659).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3301      +/-   ##
==========================================
+ Coverage   74.58%   74.62%   +0.03%     
==========================================
  Files         188      188              
  Lines        8187     8192       +5     
==========================================
+ Hits         6106     6113       +7     
+ Misses       1841     1840       -1     
+ Partials      240      239       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@anjalii-28 anjalii-28 force-pushed the fix-configmap-informer branch from acbb160 to f95d534 Compare December 16, 2025 14:07
@anjalii-28
Copy link
Author

/easycla

@dprotaso
Copy link
Member

/lgtm
/approve

Thanks for the change - let me know when you sort out the EasyCLA

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2025
@knative-prow
Copy link

knative-prow bot commented Dec 16, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anjalii-28, dprotaso

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 16, 2025
@knative-prow
Copy link

knative-prow bot commented Dec 17, 2025

New changes are detected. LGTM label has been removed.

@knative-prow knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 17, 2025
t.Fatalf("foo1.count = %v, want %d", got, want)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

I guess this was in 33ffc8e to trigger the easyCLA check.
For the future: IIRC this can also be triggered through some command like /easycla

Copy link
Author

Choose a reason for hiding this comment

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

@creydr removed this extra line, but somehow easyCLA check is not passing.
Can you help ?

Copy link
Member

Choose a reason for hiding this comment

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

In #3301 (comment) it says:

So you need to make sure your first commit (f95d534) comes from an email, linked to your GitHub user. Check for example on the linked help articles how to do it

@creydr
Copy link
Member

creydr commented Dec 17, 2025

/easycla

@anjalii-28 anjalii-28 requested a review from creydr December 17, 2025 08:22
@dprotaso
Copy link
Member

easy cla isn't satisfied - unsure if you need to sign older commits?

git rebase --signoff HEAD~N 

Generally though I thought it was tied to the email in your github account

@dprotaso
Copy link
Member

I'm not really an EasyCLA expert so you'll want to open an issue here: https://github.com/linuxfoundation/easycla

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ConfigMap informer panics with DeletedFinalStateUnknown type assertion

3 participants