Skip to content

Conversation

@jsoriano
Copy link
Member

@jsoriano jsoriano commented Jun 29, 2023

Use standard library for errors wrapping.

Most code changes done with github.com/xdg-go/go-rewrap-errors and goimports.

Steps followed to prepare this PR:

  • find . -name "*.go" -exec go run github.com/xdg-go/go-rewrap-errors@latest -w {} \;
  • find . -name "*.go" -exec goimports -w {} \;
  • go mod tidy
  • Manually replaced uses of log.Fatal(fmt.Errorf with log.Fatalf(... (not really related, but took the opportunity to fix this).
  • make check-static, and fix linting issues related to error messages.
  • Fix case with nil error where errors.Wrap would have returned nil (6993819).

@jsoriano jsoriano self-assigned this Jun 29, 2023
@jsoriano jsoriano requested a review from a team June 29, 2023 09:59
@jsoriano
Copy link
Member Author

/test

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @jsoriano

Copy link
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

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

Nice to have all these occurrences replaced, thanks!

@jsoriano jsoriano merged commit 43c68bc into elastic:main Jun 29, 2023
@jsoriano jsoriano deleted the re-wrap-errors branch June 29, 2023 15:29
Comment on lines 110 to 112
if len(node.Content) == 0 || node.Content[0].Kind != yaml.MappingNode {
return nil, errors.Wrap(err, "unexpected manifest content")
return nil, fmt.Errorf("unexpected manifest content: %w", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not right; err is nill here but this returns a non-nil error referring to err.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, good catch.

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.

4 participants