8000 Fix linter suggestions by Fedosin · Pull Request #15924 · knative/serving · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix linter suggestions #15924

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix linter suggestions #15924

wants to merge 1 commit into from

Conversation

Fedosin
Copy link
Contributor
@Fedosin Fedosin commented Jun 11, 2025 •
edited
Loading

Fixes #15755

Proposed Changes

For the specific case described here, we now check for a NotFound error and return an empty list.

Release Note

NONE

@knative-prow knative-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 11, 2025
@knative-prow knative-prow bot requested review from dprotaso and skonto June 11, 2025 22:28
@dprotaso
Copy link
Member

linter changes are already part of this PR - #15922

@dprotaso
Copy link
Member
dprotaso commented Jun 11, 2025 •
edited
Loading

/hold

can you just fix the Check NotFound error when we get certificates in this PR

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2025
@codecov Codecov
Copy link
codecov bot commented Jun 11, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 80.90%. Comparing base (c50a00d) to head (68ba69c).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/reconciler/route/route.go 0.00% 7 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #15924      +/-   ##
==========================================
- Coverage   80.98%   80.90%   -0.08%     
==========================================
  Files         210      210              
  Lines       16780    16784       +4     
==========================================
- Hits        13589    13579      -10     
- Misses       2839     2850      +11     
- Partials      352      355       +3

☔ 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.

@Fedosin Fedosin force-pushed the fix-linter-suggestions branch from edb3f60 to 68ba69c Compare June 11, 2025 23:34
@knative-prow Knative Prow
Copy link
knative-prow bot commented Jun 11, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Fedosin
Once this PR has been reviewed and has the lgtm label, please assign gauron99 for approval. For more information see the Code Review Process.

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

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 size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 11, 2025
Fedosin
Fedosin commented Jun 12, 2025
View reviewed changes
Copy link
Contributor Author
@Fedosin Fedosin left a comment

Choose a reason for hiding this comment

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

/hold cancel

Thank you for the suggestion! I kept only the fix mentioned in the issue.

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 12, 2025
dprotaso
dprotaso reviewed Jun 13, 2025
View reviewed changes
pkg/reconciler/route/route.go
@@ -381,6 +384,11 @@ func (c *Reconciler) getOrphanRouteCerts(r *v1.Route, domainToTagMap map[string]

certs, err := c.certificateLister.Certificates(r.Namespace).List(labelSelector)
if err != nil {
// In case of NotFound, return empty list
if kerrors.IsNotFound(err) {
Copy link
Member

Choose a reason for hiding this comment

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

Curious when does listing a resource return not found?

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2025
@knative-prow-robot
Copy link
Contributor

PR needs rebase.

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.

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

@dprotaso dprotaso dprotaso left review comments

@skonto skonto Awaiting requested review from skonto

Assignees
No one assigned
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

linter fails
3 participants
@Fedosin @dprotaso @knative-prow-robot
0