cluster-api icon indicating copy to clipboard operation
cluster-api copied to clipboard

Published 20 hours ago • verified publisher icon kubernetes-sigs

🌱 Use errors package of Go

Open sivchari opened this issue 1 year ago • 4 comments

What this PR does / why we need it:

Go has errors package to use Is() and As(). These API can check recursively if the err is one we want. So I use errors package to check recursively.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes #

sivchari avatar Jul 16 '24 11:07 sivchari

/assign

sivchari avatar Jul 19 '24 03:07 sivchari

I currently don't find the time to review this PR. Just one comment, we have to be really really sure this doesn't break anything accidentally (by catching more or less errors than before)

sbueringer avatar Nov 14 '24 17:11 sbueringer

I rebased and fixed some review points. Thanks.

sivchari avatar Feb 05 '25 03:02 sivchari

The list that I should write UnitTests

  • [x] handleGithubErr
  • [x] (g *gitHubRepository) getVersions
  • [x] (g *gitHubRepository) handleGithubErr
  • [x] (r *KubeadmControlPlaneReconciler) Reconcile

sivchari avatar Feb 11 '25 05:02 sivchari

Will take some time for me to get back to this. Just too much other things in my backlog.

Anyway, heads up. This PR has to be rebased once https://github.com/kubernetes-sigs/cluster-api/pull/12038 and #12037 are merged.

Some part of the changes in this PR won't be necessary anymore as I had to change the Machine controller to implement v1beta2 in 12038 (and I used errors.Is)

sbueringer avatar Mar 27 '25 15:03 sbueringer

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

This bot triages 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 PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this 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 Aug 06 '25 04:08 k8s-triage-robot

It sounds good, then if we decide to use it, we might be able to drop the direct dependence of pkg/errors completely.

sivchari avatar Aug 07 '25 11:08 sivchari

Can we please stick with using pkg/errors for this PR?

We can have the discussion on how to migrate and then make this a dedicated effort, but it should not be mixed with other changes in my opinion.

(For what it's worth, I agree with Lubomir's suggestion on how to migrate)

sbueringer avatar Aug 08 '25 10:08 sbueringer

Thx!

/lgtm /approve /test pull-cluster-api-e2e-main

sbueringer avatar Sep 15 '25 09:09 sbueringer

LGTM label has been added.

Git tree hash: 2f3a32316c42371c579f7a6cece3686d37c59a49

k8s-ci-robot avatar Sep 15 '25 09:09 k8s-ci-robot

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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 15 '25 09:09 k8s-ci-robot

Will be fixed elsewhere

/override pull-cluster-api-e2e-main

sbueringer avatar Sep 15 '25 11:09 sbueringer

@sbueringer: Overrode contexts on behalf of sbueringer: pull-cluster-api-e2e-main

In response to this:

Will be fixed elsewhere

/override pull-cluster-api-e2e-main

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.

k8s-ci-robot avatar Sep 15 '25 11:09 k8s-ci-robot