apiserver-network-proxy icon indicating copy to clipboard operation
apiserver-network-proxy copied to clipboard

Published 20 hours ago • verified publisher icon kubernetes-sigs

Simple konn-client cleanups

Open rata opened this issue 2 years ago • 11 comments

This PR doesn't make any behavior change, just cleans up the code to reduce unneeded indentations and just tries to filter out errors first for better readability.

If when you review this you think "hm, do we want to close the connection in this case instead of just continue on the loop?", I wonder the same and opened https://github.com/kubernetes-sigs/apiserver-network-proxy/issues/338 to investigate that. This PR, though, is just making the current code more readable. The bugfix (if any ends up being needed) can be discussed in that issue.

rata avatar Mar 10 '22 11:03 rata

Hi @rata. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

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/test-infra repository.

k8s-ci-robot avatar Mar 10 '22 11:03 k8s-ci-robot

/retest

rata avatar Mar 10 '22 16:03 rata

Tests are green now :)

rata avatar Mar 10 '22 16:03 rata

Friendly ping? :)

rata avatar Mar 17 '22 11:03 rata

@rata thinking we should hold on merging non bug fixes until this is done https://github.com/kubernetes-sigs/apiserver-network-proxy/issues/346, thoughts?

andrewsykim avatar Mar 17 '22 13:03 andrewsykim

@andrewsykim SGTM, thanks!

rata avatar Mar 17 '22 14:03 rata

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Jun 15 '22 15:06 k8s-triage-robot

/remove-lifecycle stale

We are waiting for the release before moving with more changes.

rata avatar Jun 16 '22 09:06 rata

I'll rebase when there is a release and we can merge this (if that is the path agreed)

rata avatar Jun 21 '22 10:06 rata

@rata Rebased to the latest master

@cheftako These changes are cosmetic in nature to improve readaiblity of the code.

Please review at your convenience :)

ipochi avatar Sep 08 '22 14:09 ipochi

@andrewsykim Addressed your feedback.

ipochi avatar Sep 09 '22 10:09 ipochi

/lgtm /approve

cheftako avatar Sep 09 '22 16:09 cheftako

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, rata

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

The pull request process is described 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

k8s-ci-robot avatar Sep 09 '22 16:09 k8s-ci-robot