-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: allow setup for go-vet, govulncheck, and go-build #270
Conversation
00d244d
to
430006f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: what cases would setups be different for build, vet, and vuln? I'm wondering if buildSetup
could simply be used for all three workflows...
@wilsonjord yes, I was wondering the same thing.
I would say |
In practice, these are all the same (at least from what I've seen so far. golangciSetup has never been used in the GeoNet codebase, so not worried about the breaking change. An attempt to make the reusable-go-apps workflow actually reusuable.
Failed the commit conformance. It's quite strict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps go-test
should be ${{ inputs.testSetup || inputs.buildSetup }}
type: string | ||
required: false | ||
description: | | ||
shell commands to setup the test environment | ||
golangciSetup: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: I don't know if anyone uses this, but if they did, this is a breaking change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked, no one uses it.
For the reusable-go-apps workflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Reference: https://github.com/GeoNet/tickets/issues/15422
In doing the above ticket^, I have found the
reusable-go-apps
workflow not very reusable at all. This change will align it more closely with its name.Tested here, works: https://github.com/GeoNet/4D-Ashfall/actions/runs/9120866234/job/25079049471