Skip to content
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

replace multierr dep with stnd lib as much as possible #11818

Closed
wants to merge 4 commits into from

Conversation

poopoothegorilla
Copy link
Contributor

@poopoothegorilla poopoothegorilla commented Jan 18, 2024

This PR aims to reduce the amount we use 3rd party pkgs

  • replaces multierr.Combine with errors.Join
  • replaces multierr.Append with errors.Join
  • replaces multierr.AppendInto with errors.Join
  • changes places where github.com/pkg/errors also exists to pkgerrors to prevent confusion with the standard library pkg

Notes:

  • didnt want to replace github.com/pkg/errors in this pr... that might need more thought and buyin

Related:

Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

Copy link
Contributor

I see that you haven't updated any README files. Would it make sense to do so?

Copy link
Contributor

@krehermann krehermann left a comment

Choose a reason for hiding this comment

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

thank you for your public service.

@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate failed

Failed condition C Reliability Rating on New Code (is worse than A)
Failed condition 48.6% 48.6% Coverage on New Code (is less than 75%)
Failed condition 14.5% 14.47% Duplicated Lines (%) on New Code (is greater than 3%)

See analysis details on SonarQube

Fix issues before they fail your Quality Gate with SonarLint SonarLint in your IDE.

Copy link
Collaborator

@samsondav samsondav left a comment

Choose a reason for hiding this comment

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

This looks fine, do we have a plan for including stacktraces in the errors though? This can sometimes be very helpful

@jmank88
Copy link
Contributor

jmank88 commented Jan 19, 2024

This looks fine, do we have a plan for including stacktraces in the errors though? This can sometimes be very helpful

We haven't chosen a direct replacement to create an error with a trace, although logging at ERROR level or above will include one.

@poopoothegorilla
Copy link
Contributor Author

This looks fine, do we have a plan for including stacktraces in the errors though? This can sometimes be very helpful

This PR should only modify Multierr, which i dont think has stack traces with errors. It doesnt touch pkg/errors which has stack traces, it just renames it

Copy link
Contributor

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 20, 2024
@poopoothegorilla poopoothegorilla deleted the jtw/rm-multierr-dep branch March 21, 2024 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants