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

Switch from fmt.Errorf to xerrors.Errorf #5425

Merged
merged 3 commits into from
Aug 30, 2021
Merged

Switch from fmt.Errorf to xerrors.Errorf #5425

merged 3 commits into from
Aug 30, 2021

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Aug 27, 2021

No description provided.

@aledbf aledbf force-pushed the aledbf/errorf branch 2 times, most recently from 26810e0 to b500cbc Compare August 27, 2021 20:38
@codecov
Copy link

codecov bot commented Aug 27, 2021

Codecov Report

Merging #5425 (37c832b) into main (4f9aa60) will increase coverage by 6.79%.
The diff coverage is 65.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5425      +/-   ##
==========================================
+ Coverage   22.98%   29.78%   +6.79%     
==========================================
  Files          11       38      +27     
  Lines        1945     6672    +4727     
==========================================
+ Hits          447     1987    +1540     
- Misses       1439     4501    +3062     
- Partials       59      184     +125     
Flag Coverage Δ
components-gitpod-cli-app 9.74% <100.00%> (?)
components-ws-daemon-app 22.98% <0.00%> (ø)
components-ws-manager-app 38.35% <69.09%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/gitpod-cli/cmd/root.go 0.00% <ø> (ø)
components/ws-daemon/pkg/content/archive.go 58.88% <0.00%> (ø)
components/ws-daemon/pkg/content/config.go 62.50% <0.00%> (ø)
components/ws-daemon/pkg/content/initializer.go 0.00% <0.00%> (ø)
components/ws-daemon/pkg/internal/session/store.go 19.38% <ø> (ø)
components/ws-manager/pkg/manager/manager_ee.go 0.00% <ø> (ø)
components/ws-manager/pkg/manager/monitor.go 8.36% <32.00%> (ø)
components/gitpod-cli/cmd/env.go 14.41% <100.00%> (ø)
components/ws-manager/pkg/manager/status.go 73.33% <100.00%> (ø)
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e039c1b...37c832b. Read the comment docs.

@csweichel
Copy link
Contributor

We started using xerrors at the time of the transition to Go 1.13. Once 1.13 was around - and with it the %w verb, Unwrap, etc. we simply did not make the switch back. One of the primary drivers was the stack trackes that xerrors provides, but the default implementation in Go does not.

In most places stack traces make sense, but in some they do not, e.g. ErrNotFound. That said, consistency is a great thing :)

Just one thing: it would be awesome if issues like this made their way through groundwork.

/lgtm
/approve no-issue

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: c9778dd6bf1a80d90c2d9ef2c1ecedd7dd0ddb7c

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csweichel

Associated issue requirement bypassed by: csweichel

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

@roboquat roboquat merged commit bc3d41a into main Aug 30, 2021
@roboquat roboquat deleted the aledbf/errorf branch August 30, 2021 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants