Skip to content
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

MAISTRA-1634: Add gen-check presubmit test for maistra/istio #55

Merged
merged 2 commits into from Jul 10, 2020

Conversation

jwendell
Copy link
Member

Builder image need new tools because of that

Builder image need new tools because of that
imagePullPolicy: Always
command:
- make
- gen-check
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. I actually recently added targets to run the code-gen on our additional ServiceMesh bits. I wanted to keep the changes to the upstream Makefile minimal though, so it's not run as part of the normal make gen target. Can we maybe have this bit read:

- make
- maistra-gen-k8s-client
- gen-check

That should catch our stuff as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this target just generate files or it's a check that will fail if the PR author didn't generate them? Note that upstream has gen and gen-check. The latter is meant to fail and to be used in prow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This target just generates the files, but the gen-check target works by first running gen then running a script that checks that the git working directory is clean. So this should be enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, ok, will add that

@maistra-bot maistra-bot merged commit 5cff2c9 into maistra:main Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants