-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix an issue with verify_client.sh #805
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,12 +35,10 @@ mkdir -p "${TMP_DIFFROOT}" | |
cp -a "${DIFFROOT}"/* "${TMP_DIFFROOT}" | ||
|
||
make clientset | ||
find "${TMP_DIFFROOT}" -name *.bazel -delete | ||
|
||
echo "diffing ${DIFFROOT} against freshly generated codegen" | ||
ret=0 | ||
diff -Naupr "${DIFFROOT}" "${TMP_DIFFROOT}" -x '*.bazel' -x 'BUILD' || ret=$? | ||
cp -a "${TMP_DIFFROOT}"/* "${DIFFROOT}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @detiber i think this is line that causing issue you are seeing (correct me if not the same issue), what i found is: we don't need to copy those clientset files from |
||
if [[ $ret -eq 0 ]] | ||
then | ||
echo "${DIFFROOT} up to date." | ||
|
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.
Why not keep this with
generate
? Bazel files might need to be updated aftergo generate
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.
ok, we need it at both places to make Makefile target idempotent, the ci script
verify_clientset.sh
call make clientset, i will update the PR, thanksThere 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 was seeing issues with calling
gazelle
at the end ofclientset
where it was generating the wrong paths in the bazel build files, which was the reason for moving it to thegenerate
target.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.
@detiber i replied in another code line thread, thanks