🌱 Add CI to check if golang version updated - #1264
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1264 +/- ##
==========================================
+ Coverage 76.18% 76.51% +0.33%
==========================================
Files 40 40
Lines 2330 2363 +33
==========================================
+ Hits 1775 1808 +33
+ Misses 398 389 -9
- Partials 157 166 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
tmshort
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
| IFS='.' ver=(${whole}) | ||
| IFS="${OLDIFS}" | ||
|
|
||
| if [ ${#ver[*]} -eq 2 ] ; then |
There was a problem hiding this comment.
I'm wondering if we could simplify this block by front loading the precondition that |MAX_VER| >= |ver|, e.g.
# MAX_VER can only have a length of 2 or 3
if [ ${#ver[*]} -gt ${#MAX_VER[*]} ]; then
echo "Badly formatted golang version ${whole} in ${file} (expecting at most ${#MAX_VER[*]} version components)"
return 1
fi
if [ ${ver[0]} -gt ${GO_MAJOR} ]; then
echo "Bad golang version ${whole} in ${file} (expected ${GO_VER} or less)"
return 1
fi
if [ ${ver[1]} -gt ${GO_MINOR} ]; then
echo "Bad golang version ${whole} in ${file} (expected ${GO_VER} or less)"
return 1
fi
if [ ${#ver[*]} -eq 3 ] ; then
if [ ${ver[1]} -eq ${GO_MINOR} -a ${ver[2]} -gt ${GO_PATCH} ]; then
echo "Bad golang version ${whole} in ${file} (expected ${GO_VER} or less)"
return 1
fi
fi
echo "Version ${whole} in ${file} is good"
return 0
There was a problem hiding this comment.
At this point, we actually know that MAX_VER has 3 elements; because it's retrieved from our go.mod file that has a 3 element version. And there's definitely some simplification that can be done.
There was a problem hiding this comment.
I don't think go in go.mod has to have 3 elements. Pretty sure go <major>.<minor> is also allowed.
| fi | ||
| done | ||
|
|
||
| for f in $(find . -name "*.mod"); do |
There was a problem hiding this comment.
would this break if some dependency went from 1.22.1. to 1.22.2?
would it break if we added a new dependency or bingo tool?
There was a problem hiding this comment.
oh - the go.mod for the dependencies doesn't seem to get pulled down in vendor. Though, dependencies can include mod files, e.g. from catd:
find . -name '*.mod'
./.bingo/go.mod
./.bingo/kustomize.mod
./.bingo/setup-envtest.mod
./.bingo/bingo.mod
./.bingo/controller-gen.mod
./.bingo/golangci-lint.mod
./.bingo/goreleaser.mod
./.bingo/kind.mod
./.bingo/operator-sdk.mod
./.bingo/opm.mod
./.bingo/crd-ref-docs.mod
./go.mod
./vendor/github.com/klauspost/compress/s2sx.mod
There was a problem hiding this comment.
There should be no vendor directory, if so, that's a another problem.
Yes, it's supposed to check .bingo files
Yes, it's supposed to notify us of all go version change to go.mod-type files - but it's just a warning, not a required CI
| if ! check_version ${v} ${f}; then | ||
| RETCODE=1 | ||
| fi | ||
| old=$(git grep -ohP '^go .*$' "${BASE_REF}" -- "${f}") |
There was a problem hiding this comment.
This regex stipulates ONE space after "go"... (beware of fragile regex's)
This is a nit, as this should always hold :-)
There was a problem hiding this comment.
Actually it stipulates one space after "go", then ANYTHING, including spaces. Since we're not "capturing" the version separately, it doesn't matter.
There was a problem hiding this comment.
It's managed by go mod, so I would hope it always holds.
Signed-off-by: Todd Short <tshort@redhat.com>
|
I made the output a little less noisier. |
5 participants
Description
Reviewer Checklist