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

GRPC Apis for Corresponding HTTP APIs #369

Merged
merged 7 commits into from
Jun 23, 2024

Conversation

Prashant-Dwivedi-08-01
Copy link
Contributor

@Prashant-Dwivedi-08-01 Prashant-Dwivedi-08-01 commented Jun 7, 2024

Hey @stefanprodan
This PR is in the continuation of our previous effort to include the gRPC APIs for podinfo to extend it's application.
Previous Approved PR: #322
We introduced few APIs last time to test the stability of Podinfo with those. Now we are ready with remaining set of APIs.

Please have a look at this PR that includes the remaining set of APIs.
Thank You

Fix: #318

Copy link
Owner

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

@Prashant-Dwivedi-08-01 can you please document all added functions in the main README under gRPC API? Thanks

@Prashant-Dwivedi-08-01
Copy link
Contributor Author

@stefanprodan As per your comment, I have added description of all the gRPC APIs in the main README, you can review it once.
Apart from this, I have a request that if you can look once into why one of the checks is failing in GitHub Actions.
I am getting this error "no language version declared in module.cue" under "Verify CUE formatting".
It asks for adding the language version, but I am not sure why we should edit module.cue file.
Linking it here directly: https://github.com/stefanprodan/podinfo/actions/runs/9631176029/job/26562620984

Please see if you can help. Thanks

@stefanprodan
Copy link
Owner

Please rebase with the master branch, CI should be fixed.

@Prashant-Dwivedi-08-01
Copy link
Contributor Author

@stefanprodan thanks for that recommendation. CI is fixed. You can consider reviewing once and approving the PR. Thanks

logger *zap.Logger
}

// SayHello implements helloworld.GreeterServer
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this and the other lines below of commented code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did that! Removed unnecessary comments and whitespaces in code.
Please Check Once. Thanks

@stefanprodan
Copy link
Owner

Can you please run go fmt ./... and commit any changes.

Signed-off-by: Prashant Dwivedi <[email protected]>
@Prashant-Dwivedi-08-01
Copy link
Contributor Author

Can you please run go fmt ./... and commit any changes.

@stefanprodan did that!

Copy link
Owner

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @Prashant-Dwivedi-08-01 🏅

@stefanprodan stefanprodan added the enhancement New feature or request label Jun 23, 2024
@stefanprodan stefanprodan merged commit 752950c into stefanprodan:master Jun 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding gRPC apis functionality
2 participants