Skip to content

chore: test with go 1.20 and cleanup tooling #444

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

Merged
merged 5 commits into from
Apr 19, 2023
Merged

chore: test with go 1.20 and cleanup tooling #444

merged 5 commits into from
Apr 19, 2023

Conversation

danez
Copy link
Contributor

@danez danez commented Apr 18, 2023

Added go 1.20 to the test matrix.

This gets rid of gobin, as it is not needed anymore, instead go run can be used. Also richgo is removed (for fancy test output) because I haven't seen this in any of our other repos, and even the author recommends not using it :)

I had to update swagger-go from 0.23 to 0.24, because 0.23 wouldn't work with go run. 0.24 introduces case-insensitive enums, hence the changes to the generated models. I tried updating to the latest version (0.30), but then all generated models are very different, if we want to update we should maybe do it in steps.

Also remove the deprecated labeler job.

Fixes #416

@netlify
Copy link

netlify bot commented Apr 18, 2023

Deploy Preview for open-api ready!

Name Link
🔨 Latest commit a57ff8a
🔍 Latest deploy log https://app.netlify.com/sites/open-api/deploys/643fdcfc0ce29a00081d3bf1
😎 Deploy Preview https://deploy-preview-444--open-api.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@danez danez self-assigned this Apr 18, 2023
@github-actions github-actions bot added the type: chore work needed to keep the product and development running smoothly label Apr 18, 2023
@danez danez removed the type: chore work needed to keep the product and development running smoothly label Apr 18, 2023
@danez danez marked this pull request as ready for review April 18, 2023 13:10
@danez danez requested a review from a team as a code owner April 18, 2023 13:10
mraerino
mraerino previously approved these changes Apr 18, 2023
Copy link
Contributor

@mraerino mraerino left a comment

Choose a reason for hiding this comment

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

nice! nothing blocking

@@ -33,10 +33,10 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, macOS-latest, windows-latest]
go_version: [1.17.x, 1.18.x, 1.19.x]
go_version: [1.17.x, 1.18.x, 1.19.x, 1.20.x]
Copy link
Contributor

Choose a reason for hiding this comment

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

i think you can remove 1.17 and maybe also 1.18 as i think it would be fine to support 2 recent versions only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay changed.

runs-on: ${{ matrix.os }}
steps:
- name: Install Go ${{ matrix.go }}
- name: Install Go ${{ matrix.go_version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -10,13 +10,9 @@ require (
github.com/go-openapi/strfmt v0.19.11
github.com/go-openapi/swag v0.19.12
github.com/go-openapi/validate v0.20.0
github.com/go-swagger/go-swagger v0.23.0
Copy link
Contributor

Choose a reason for hiding this comment

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

i think you need to keep this dependency so that the go run for the go:generate instructions work.

the usual advice on how to do this is by creating a package called tools that has this:

package tools

import (
  _ "github.com/go-swagger/go-swagger"
)

Copy link
Contributor Author

@danez danez Apr 18, 2023

Choose a reason for hiding this comment

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

if go run pkg@version is used then the go module is compiled and run without it being installed or affecting the current module. That is what I did. If you prefer I can change that back to what you are suggesting.

Copy link
Contributor

Choose a reason for hiding this comment

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

if go run pkg@version is used then the go module is compiled and run without it being installed or affecting the current module. That is what I did. If you prefer I can change that back to what you are suggesting.

ah good point. i'd prefer having it in go.mod so renovate can update it though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@danez danez requested a review from mraerino April 19, 2023 12:22
@danez danez merged commit a622fcf into master Apr 19, 2023
@danez danez deleted the update branch April 19, 2023 12:34
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.

Remove gobin in makefile in favor of plain go
2 participants