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

Update Golang & GHAs #92

Merged
merged 4 commits into from
May 21, 2024
Merged

Update Golang & GHAs #92

merged 4 commits into from
May 21, 2024

Conversation

brenank
Copy link
Contributor

@brenank brenank commented May 21, 2024

No description provided.

@brenank brenank force-pushed the brenank/update-golang branch from 838730e to 62aec0c Compare May 21, 2024 04:16
@brenank brenank marked this pull request as ready for review May 21, 2024 04:17
@arran4
Copy link
Owner

arran4 commented May 21, 2024

Seems you have removed the matrix test, which was important as we have had issues where tests failed on one specific OS before. Version not so much but it's nice to have

@brenank brenank force-pushed the brenank/update-golang branch from 35baa17 to 1076d91 Compare May 21, 2024 04:46
Copy link
Owner

@arran4 arran4 left a comment

Choose a reason for hiding this comment

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

Question

calendar_fuzz_test.go Outdated Show resolved Hide resolved
@arran4
Copy link
Owner

arran4 commented May 21, 2024

@brenank This seems to be the manifests file it uses: https://github.com/actions/go-versions/blob/main/versions-manifest.json

@arran4
Copy link
Owner

arran4 commented May 21, 2024

https://github.com/actions/go-versions/blob/74d3ad35e25603a981b7fed05ea4103d03cb7bbb/versions-manifest.json#L3108

"version": "1.13.15",

Should work that's there.

(As per the go.mod file.)

The latest 14 is:

    "version": "1.14.15",

So let's try that. I suspect something is wrong with the action. I feel like these should be working.

@brenank brenank force-pushed the brenank/update-golang branch from 19cbcc1 to 0836757 Compare May 21, 2024 05:17
@brenank brenank marked this pull request as draft May 21, 2024 05:18
@arran4
Copy link
Owner

arran4 commented May 21, 2024

Some do seem to work
image

@brenank brenank force-pushed the brenank/update-golang branch from 0836757 to a13cdaa Compare May 21, 2024 05:43
@brenank brenank force-pushed the brenank/update-golang branch 2 times, most recently from c1109e9 to 717fff0 Compare May 21, 2024 05:53
@brenank brenank force-pushed the brenank/update-golang branch from 717fff0 to 022aeb0 Compare May 21, 2024 05:54
@brenank brenank force-pushed the brenank/update-golang branch from 022aeb0 to 909bd3b Compare May 21, 2024 05:58
@brenank
Copy link
Contributor Author

brenank commented May 21, 2024

Seems you have removed the matrix test, which was important as we have had issues where tests failed on one specific OS before. Version not so much but it's nice to have

Interesting! I wonder what that case would be.

Alright made some adjustments based of the feedback. Learned a bit about supporting older versions of go, and dropped the go.mod/package changes.

The reason the build was failing was github recently introduced macos-14 which runs on the ARM64 architecture. Only later versions of go in actions/go-versions have this go binary. Changing the tests to pin to macos-13 solves the issue.

@brenank brenank marked this pull request as ready for review May 21, 2024 06:13
@arran4
Copy link
Owner

arran4 commented May 21, 2024

Thanks. Great detective work!

Copy link
Owner

@arran4 arran4 left a comment

Choose a reason for hiding this comment

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

LGTM. I bet you macos-13 breaks again at some point in the future. Will probably have to consider adding a if step. I will have to add more to the contributing.md file at some point. Partial contents can be misleading

@arran4 arran4 merged commit f044c04 into arran4:master May 21, 2024
16 checks passed
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.

2 participants