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 to go modules #3

Closed
wants to merge 2 commits into from
Closed

Switch to go modules #3

wants to merge 2 commits into from

Conversation

thaJeztah
Copy link

Splitting into two commits for a clearer history;

  1. add go.mod and vendor upstream pkg/errors v0.8.0

    This patch replaces the vendor.conf with a go.mod, and re-vendors the pkg/errors files from upstream. This commit is mostly to preserve history of the local changes that were made to the pkg/errors package (which appear to have been made for debugging purposes). Setting the minimum go version in go.mod to v1.7 for this commit, which matches the lowest version that's testet against in CI.

  2. go.mod: update pkg/errors v0.9.1 and remove local vendor

    raising the minimum go version in go.mod 1.12, to fetch the dependency module. Updated travis to test against 1.12 (minimum version) and 1.15 (current stable).

I'll also open a follow-up to remove the dependency on pkg/errors, however that requires Go 1.13 or up, so perhaps you'd want to keep that separate (and tag a separate version for that change)

@thaJeztah
Copy link
Author

@cyphar @kolyshkin PTAL

(follow-up is in #4)

@kolyshkin
Copy link
Contributor

So, why go 1.12 and not, say, 1.14 (the oldest currently supported release), or 1.13 (the most fresh unsupported release)?

@thaJeztah
Copy link
Author

So, why go 1.12 and not, say, 1.14

Trying to keep the go.mod to reflect the actual minimum required version of Go (although I don't think go modules so far enforce the minimum version specified)

This patch replaces the vendor.conf with a go.mod, and re-vendors
the pkg/errors files from upstream.

This commit is mostly to preserve history of the local changes that
were made to the pkg/errors package (which appear to have been made
for debugging purposes).

Setting the minimum go version in go.mod to v1.7 for this commit,
which matches the lowest version that's testet against in CI.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
raising the minimum go version in go.mod 1.12, to fetch the
dependency module.

Updated travis to test against 1.12 (minimum version) and
1.15 (current stable).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comment on lines -132 to -138
// Debug sets whether or not debugging is enabled. If enabled, then formatting
// an error wrapped by pkg/error with "%v" will return a full stack trace. This
// allows you to dynamically decide whether to output stack traces (without
// having to use "%+v" indiscriminately).
func Debug(value bool) {
debugEnabled.Set(value)
}
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure if you noticed this, but this is actually a patched version of pkg/error. Though we don't really need this anymore, so no worries.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I noticed; I did the first commit separately for that;

This commit is mostly to preserve history of the local changes that were made to the pkg/errors package (which appear to have been made for debugging purposes). (...)

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, my fault for not reading 🙇.

Copy link
Author

Choose a reason for hiding this comment

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

No worries! It was a good catch 👍

cyphar added a commit that referenced this pull request Jun 15, 2021
Kir Kolyshkin (3):
  Ditch pkg/errors, use native error (un)wrapping
  go.mod: add
  travis-ci: update Go versions

LGTM: cyphar
Closes #7 #4 #3
@cyphar
Copy link
Owner

cyphar commented Jun 15, 2021

Closing in favour of #7.

@cyphar cyphar closed this Jun 15, 2021
@thaJeztah thaJeztah deleted the gomod branch June 15, 2021 09:26
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.

3 participants