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

Migrate from multierr.Combine to errors.Join and update go version #708

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Aug 15, 2023

Fixes #704

This also updates the go version support window from [1.19,1.20] to [1.20,1.21].

From the definition of errors.Join:

Join returns an error that wraps the given errors. Any nil error values are discarded. Join returns nil if every value in errs is nil. The error formats as the concatenation of the strings obtained by calling the Error method of each element of errs, with a newline between each string.

A non-nil error returned by Join implements the Unwrap() []error method.

@dashpole dashpole requested a review from a team as a code owner August 15, 2023 15:36
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #708 (bba89c4) into main (c915721) will increase coverage by 0.20%.
The diff coverage is 59.45%.

@@            Coverage Diff             @@
##             main     #708      +/-   ##
==========================================
+ Coverage   69.39%   69.59%   +0.20%     
==========================================
  Files          36       36              
  Lines        4735     4717      -18     
==========================================
- Hits         3286     3283       -3     
+ Misses       1298     1286      -12     
+ Partials      151      148       -3     
Files Changed Coverage Δ
exporter/collector/logs.go 45.48% <0.00%> (+0.50%) ⬆️
exporter/collector/metrics.go 73.05% <50.00%> (+0.32%) ⬆️
exporter/metric/metric.go 69.70% <73.07%> (+0.98%) ⬆️
exporter/trace/trace.go 90.16% <100.00%> (-0.75%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

LGTM if you can get the tests to pass

@dashpole
Copy link
Contributor Author

I'm going to update the go version (errors.Join was added in 1.20). We support the latest 2 versions, so that should be OK

@dashpole dashpole changed the title Migrate from multierr.Combine to errors.Join Migrate from multierr.Combine to errors.Join and update go version Aug 15, 2023
@dashpole dashpole merged commit fff463f into GoogleCloudPlatform:main Aug 15, 2023
@dashpole dashpole deleted the errors_join branch August 15, 2023 17:30
@anuraaga
Copy link
Contributor

Hi @dashpole - it looks like this was merged and released with go 1.21. Given your last comment, wondering if this was supposed to be go 1.20?

@dashpole
Copy link
Contributor Author

@anuraaga I went ahead and updated to 1.21, since we support the latest 2 versions of go. Did this cause issues for you?

@anuraaga
Copy link
Contributor

I saw a warning when compiling with 1.20, not sure if there was something special about my build setup. But AFAIK the intent of that flag is for libraries to always set the version in go.mod to the minimum supported so the Go compiler prevents as much as possible incompatibilities when compiling with the older version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate from multierr to errors.Join
3 participants