Add CI workflow #2
Conversation
der-eismann
force-pushed
the
add-ci-workflow
branch
8 times, most recently
from
e82c88b
to
8e2b17d
Compare
There was a problem hiding this comment.
LGTM! Can't suggest improvements since you are the devops guy here :) but what I see is correct.
Let's just wait for @GZGavinZhao before merging.
| - name: Setup Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.22' |
There was a problem hiding this comment.
Wouldn't it be better to use https://github.com/actions/setup-go?tab=readme-ov-file#getting-go-version-from-the-gomod-file ? That way, we test our target version AND we don't have to remember to update the CI code.
There was a problem hiding this comment.
Yes and no. I was wondering if the best way would be to put the oldest working version in the go.mod so library users are not forced to use the newest Go or actually go for the newest release. What's your point on that?
There was a problem hiding this comment.
Ah yes, I agree with that. We should keep the Go version inside the go.mod file as lower as possible. But I think this is a different problem than selecting the Go version inside the CI. Anyway, I'm not forcing that, if you're more comfortable with hard-coding the Go version in ci.yml :)
There was a problem hiding this comment.
LGTM! I would personally like to see golangci-lint enabled as a linter (action here]), but it might be better to do that in a separate PR to also fix any issues it finds at the same time.
|
At this point I'd merge the two Codeowners commit and merge the PR. @der-eismann thought? Can you merge the commits for a cleaner history? |
der-eismann
force-pushed
the
add-ci-workflow
branch
from
e77658a
to
9c632a6
Compare
No description provided.