-
Notifications
You must be signed in to change notification settings - Fork 2
Added more code validation checkts to CI #71
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
Conversation
.github/workflows/ci.yml
Outdated
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'm not sure, but i'm guessing that the issue you saw might be worked around by putting quotes around this expression or something to that effect. did you try any other ways to format this?
|
any updates? you'd mentioned you were going to look at removing the validate vendor check |
|
@clnperez update the change, commented vendor check for now |
|
You may need to pull in the latest upstream to get these checks to pass. I just tried re-running them and no-go. |
|
@bkhadars any way you can get this in soon? i'm kind of waiting on it before we check in too many other PRs so we can have these checks. |
|
Raised PR for verifying codegen script, I think that PR should fix the issue |
|
I think you have to rebase one more time to pick up the codegen fixes. |
|
@bkhadars ping |
|
@clnperez I have rebased the code to latest, but the checks are still failing as go vet list errors from the platform validation testcases... We should fix those platform validation test cases.. I have commented out those testcases for time being and raised the New PR with the other vet changes |
|
@clnperez I have rebased the changes and vet is failing as the vendor changes are required... |
|
the codegen test exits with errors but doesn't fail the check: and it should be in its own job like the rest.
If we check in all the vendor changes this ups our likelihood for merge conflicts exponentially. We just can't check in those since we pull in upstream changes all the time. If you need the vendored files to be there just do what is done for the build, and run |
|
looks like the codegen issue is actually a problem with the test itself (see openshift#5406 (comment)) so we can leave that bit as-is |
|
in trying to understand what was going on with your PR I ended up just creating my own b/c that was the only way to test it, so, we can just close this one. thanks for the other fixes and getting this started! |
No description provided.