-
Notifications
You must be signed in to change notification settings - Fork 117
DNM: Release v0.37.2 #2319
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
DNM: Release v0.37.2 #2319
Conversation
Add nil check for Kubernetes client in EmitMessage to prevent panic when client is uninitialized. This fixes a crash that occurs during error handling in GitLab provider when 401 errors are encountered. The EventEmitter now gracefully handles three nil scenarios: - nil repo: skips event creation, continues logging - nil logger: skips logging, continues event creation - nil client: skips event creation, continues logging Added comprehensive test coverage for nil client scenario to match existing tests for nil repo and nil logger cases. Fixes crash: runtime error: invalid memory address or nil pointer dereference at pkg/events/emit.go:36 Jira: https://issues.redhat.com/browse/SRVKP-8910 Assisted-by: Claude-Sonnet-4.5 (via Claude Code) Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Summary of ChangesHello @zakisk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical fix to prevent a nil pointer panic within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a fix to prevent a nil pointer panic in EventEmitter when its client is not initialized. The change adds a nil check for the client before attempting to emit a Kubernetes event. The accompanying test changes are well-structured, adding a new test case to ensure the fix works as expected and doesn't cause a panic. I've suggested a small improvement to the new test case to also verify the logging behavior, which would make the test more comprehensive. Overall, this is a good fix that improves the robustness of the event emitter.
| { | ||
| name: "nil client doesn't cause panic", | ||
| repo: &v1alpha1.Repository{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "test-repo", | ||
| Namespace: "test-ns", | ||
| }, | ||
| Spec: v1alpha1.RepositorySpec{}, | ||
| }, | ||
| message: "info-message", | ||
| logLevel: zap.InfoLevel, | ||
| expectEvent: false, | ||
| expectedType: v1.EventTypeNormal, | ||
| useNilClient: true, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case correctly verifies that a nil client doesn't cause a panic and that no Kubernetes event is emitted. However, the EmitMessage function is also expected to log the message regardless of whether an event is emitted. To make this test more comprehensive, consider asserting that the message is indeed logged using the zapobserver.
Converted deprecated `// +build` directives to the modern `//go:build` format across several files. This standardized the build constraint declarations. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Replace deprecated binary field with binaries array syntax in homebrew_casks configuration to resolve GoReleaser deprecation warning. Signed-off-by: Akshay Pant <akshay.akshaypant@gmail.com>
Deploying pipelines-as-code with
|
| Latest commit: |
8195b9d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b35b7b0b.pipelines-as-code.pages.dev |
| Branch Preview URL: | https://release-v0-37-2.pipelines-as-code.pages.dev |
|
/retest linters |
Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
|
/retest linters |
This release contains:
3725c38 chore(goreleaser): use binaries field in homebrew
ac8e45c refactor: Migrate build tags from +build to //go:build directives
5fbebdf fix: prevent nil pointer panic in EventEmitter