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

Verify make gen-all-sdk-grpc has been run #3303

Closed
zmerlynn opened this issue Aug 1, 2023 · 4 comments
Closed

Verify make gen-all-sdk-grpc has been run #3303

zmerlynn opened this issue Aug 1, 2023 · 4 comments
Labels
help wanted We would love help on these issues. Please come help us! kind/cleanup Refactoring code, fixing up documentation, etc

Comments

@zmerlynn
Copy link
Collaborator

zmerlynn commented Aug 1, 2023

#3302 exposes an issue in our CI: We don't verify that make gen-all-sdk-grpc has been run. We should enforce that any SDK API (proto, etc) changes regenerate the gRPC, and we should enforce we can still build with the new generation.

cc @markmandel

@zmerlynn zmerlynn added kind/bug These are bugs. kind/cleanup Refactoring code, fixing up documentation, etc help wanted We would love help on these issues. Please come help us! and removed kind/bug These are bugs. labels Aug 1, 2023
@markmandel
Copy link
Member

@Kalaiselvi84 this may be a good job for you, since you've done many of the other ones.

@Kalaiselvi84
Copy link
Contributor

I need to add below step in agones/cloudbuild.yaml, when there are any changes to the SDK API, and the system should still be buildable with the updated gRPC code?

  • name: "make-docker"
    id: gen-all-sdk-grpc
    waitFor:
    • pull-build-image
      dir: "build"
      args: ["gen-all-sdk-grpc"]

@zmerlynn
Copy link
Collaborator Author

zmerlynn commented Aug 2, 2023

@Kalaiselvi84 My thinking was that this would be similar to how test-gen-api-docs works, a CI check. It's actually easier than gen-api-docs because at least as far as I can tell gen-all-sdk-grpc is stable - I.e. you can just run it and see if there's a diff. It's possible this is not the case over time (I.e. something may be non-hermetic in the process), but we can work through that later.

@zmerlynn
Copy link
Collaborator Author

This was done in #3349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We would love help on these issues. Please come help us! kind/cleanup Refactoring code, fixing up documentation, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants