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

ioutil + jwt deprecations #120

Merged
merged 5 commits into from
May 29, 2024
Merged

Conversation

kfcampbell
Copy link
Contributor

@kfcampbell kfcampbell commented May 21, 2024

I know #111 exists but I figured it'd be worth proposing this PR without the google/go-github bump in case it's worth taking by itself.

The Go standard library's ioutil package is deprecated. This updates those usages to either io or os package functions.

It also updates a test-only usage of the jwt.StandardClaims to jwt.RegisteredClaims.

BREAKING CHANGE: Since these io and os package functions were introduced in Go 1.16, this PR updates the go.mod file's go line to version 1.16.

@kfcampbell kfcampbell changed the title ioutil deprecations ioutil + jwt deprecations May 21, 2024
@jamestelfer
Copy link

All these methods were introduced in Go 1.16 -- should go.mod be advanced to match this?

@kfcampbell
Copy link
Contributor Author

Great catch @jamestelfer! I've done so, and marked the PR as a breaking change in the writeup above.

Copy link
Collaborator

@wlynch wlynch left a comment

Choose a reason for hiding this comment

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

LGTM!

One thing I want to double check w/ @bradleyfalzon before submitting - how do you feel about bumping go directives? Technically this is a breaking change for older Go versions, but most people are unlikely to hit (upstream only supports last 2 versions, so as of now >= 1.21). Do you think this warrants a major version bump?
I'm okay with having minor releases track the oldest upstream supported version - wdyt?

@bradleyfalzon
Copy link
Owner

Yeah, this would be breaking the build for some users, but wouldn’t require a code change. The remediation for the user is increasing the version of go which shouldn’t be a breaking change whilst we’re just increasing minor version change.

So I think it’s fine to not be a major version change, especially as mentioned, we’re only requiring a less ancient release.

@wlynch wlynch merged commit 289c613 into bradleyfalzon:master May 29, 2024
1 check passed
@kfcampbell kfcampbell deleted the deprecations branch June 3, 2024 20:24
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.

None yet

4 participants