WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@alex5517
Copy link
Contributor

No description provided.

@alex5517 alex5517 requested a review from KimNorgaard March 19, 2025 17:38
KimNorgaard
KimNorgaard previously approved these changes Mar 19, 2025
Copy link
Member

@langecode langecode left a comment

Choose a reason for hiding this comment

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

With the risk of repeating a conversation that had already been done - I have added a few comments.

if: inputs.run-benchmarks
run: go test -bench=./...

- name: Install GoReleaser
Copy link
Member

Choose a reason for hiding this comment

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

Are we using GoRelease for building container images? Or should this be optional? Do we need a simple go build ./... as part of the pipeline also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@langecode
I think we could use it for container images yes, and GoReleaser does run build ./.. and CI would also had done this.
Otherwise consumers can also customize as they see fit using the .goreleaser file.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so we outsource go build ./... to GoReleaser regardless of the project containing a .goreleaser configuration or not. Guess that makes sense then - just not very obvious reading the pipeline that GoReleaser will run this

langecode
langecode previously approved these changes Mar 24, 2025
Copy link
Member

@langecode langecode left a comment

Choose a reason for hiding this comment

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

I think it looks pretty good 👍

Only thing that I think we could add (but we could also do that later) is posting the test-results to the Github action for more easy overview if tests are failing. I think there is also integrations for publishing test-coverage. But we should possibly let this rest and do it as a separate thing sometime.

if: inputs.run-benchmarks
run: go test -bench=./...

- name: Install GoReleaser
Copy link
Member

Choose a reason for hiding this comment

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

Ok, so we outsource go build ./... to GoReleaser regardless of the project containing a .goreleaser configuration or not. Guess that makes sense then - just not very obvious reading the pipeline that GoReleaser will run this

Signed-off-by: Alexander Soelberg Heidarsson <[email protected]>
@alex5517 alex5517 merged commit a993ac2 into main Apr 4, 2025
@alex5517 alex5517 deleted the feat/improve-go-actions branch April 4, 2025 03:14
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.

5 participants