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

github: Fix CI pipelines #2266

Merged
merged 7 commits into from
Jul 3, 2022
Merged

github: Fix CI pipelines #2266

merged 7 commits into from
Jul 3, 2022

Conversation

tmc
Copy link
Contributor

@tmc tmc commented Jul 2, 2022

This fixes the CI pipelines:

Integration Security Lint Test

This modernizes the workflows uses a curl retry to wait for the servers being up.

@coveralls
Copy link

coveralls commented Jul 2, 2022

Coverage Status

Coverage remained the same at 75.097% when pulling 146ddaf on tmc:tmc-fix-ci into 8481457 on 99designs:master.

Copy link
Collaborator

@StevenACoffman StevenACoffman left a comment

Choose a reason for hiding this comment

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

Did not testing on Go 1.17 or Go 1.18 cause problems?

go-version: ${{ matrix.go }}
- uses: actions/setup-node@v3
with:
node-version: 14
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make this a root env variable so we can only maintain it in one place?

jobs:
integration:
strategy:
matrix:
go: [1.16, 1.17, 1.18]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Testing on all three versions will take longer and cost more. Do you think passing on Go 1.16 is likely to fail on Go 1.17 or Go 1.18?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought GitHub was pretty generous with the open source use of github actions.

Copy link
Collaborator

@StevenACoffman StevenACoffman Jul 3, 2022

Choose a reason for hiding this comment

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

99designs has exceeded the free tier, but it's unclear to me as an external maintainer to how much this repository contributed to that usage, compared to their other open source repositories, or their private GitHub action usage. Since it's not my money, I try to only use what I need. I think your revision where you are testing oldest supported (Go 1.16) and newest supported (Go 1.18) is a great compromise. Thanks!

Copy link
Member

@mtibben mtibben Jul 4, 2022

Choose a reason for hiding this comment

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

I would prefer the last 2 versions of Go, to align with the Go EOL policy

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mtibben I agree with you that is the goal, but we at Khan are using Go 1.16 until GCP AppEngine supports a newer Go version, and I can't help maintain this project if it is beyond that. GCP tells us they plan on skipping Go 1.17 and releasing support for Go 1.18 in H2 of 2022 (so July-Dec 2022) but haven't increased that. As a result, we previously agreed to keep gqlgen supporting the same Go version as Khan uses in prod (and newer).

Copy link
Member

Choose a reason for hiding this comment

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

yeah fair enough 👍

@tmc tmc force-pushed the tmc-fix-ci branch 2 times, most recently from 14bbb18 to 0bd22d1 Compare July 2, 2022 16:58
@tmc tmc marked this pull request as ready for review July 2, 2022 17:54
@tmc tmc force-pushed the tmc-fix-ci branch 2 times, most recently from ff72646 to 9644f56 Compare July 2, 2022 20:14
@StevenACoffman
Copy link
Collaborator

This is great! Thanks so much for helping to avoid the flaky failures we've been experiencing!

@StevenACoffman StevenACoffman merged commit b8497f5 into 99designs:master Jul 3, 2022
@tmc tmc deleted the tmc-fix-ci branch July 5, 2022 00:09
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