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

feat: add fields with proto3_optional #380

Merged
merged 5 commits into from
May 15, 2020

Conversation

noahdietz
Copy link
Collaborator

@noahdietz noahdietz commented May 14, 2020

Adds several primitive fields to google.showcase.v1.User that are marked with proto3_optional. Adds a method_signature with all of the proto3_optional fields.
Upgrades protoc version to v3.12.0-rc-2 in ci.

Chore: elide struct type name in slices in identity_service_test.go to remove linter warnings
Chore: disable kotlin-smoke-test job, it is not maintained and doesn't support proto3_optional
Chore: remove go-smoke-test job because it's redundant, gapic-generator-go is run in the compile_protos job
Chore: disable python-smoke-test and typescript-smoke-test jobs until they support proto3_optional, tracked by #382 and #383 respectively

Note: PR includes regenerated protobuf/gRPC API and CLI (no changes in Go GAPIC).

cc: @AlanGasperini @hkdevandla

@alexander-fenster
Copy link
Contributor

re: TypeScript build failure - it requires googleapis/gapic-generator-typescript#460 that I will merge within a day or two.

Copy link

@chrisdunelm chrisdunelm left a comment

Choose a reason for hiding this comment

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

LGTM
Although as a European, I'm disappointed by height being measured in feet ;)

noahdietz added 2 commits May 15, 2020 15:02
* gapic-generator-kotlin is unmaintained and the work to
support proto3_optional is unplanned
* go-smoke-test is unnecessary because gapic-generator-go
is used in the compile_protos step to regenerate the
client and CLI on every PR
@codecov
Copy link

codecov bot commented May 15, 2020

Codecov Report

Merging #380 into master will decrease coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
- Coverage   99.61%   99.42%   -0.19%     
==========================================
  Files          13       13              
  Lines        1036     1048      +12     
==========================================
+ Hits         1032     1042      +10     
- Misses          2        3       +1     
- Partials        2        3       +1     
Impacted Files Coverage Δ
server/services/identity_service.go 100.00% <100.00%> (ø)
server/services/messaging_service.go 99.48% <0.00%> (-0.52%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e6baae...08771dc. Read the comment docs.

@noahdietz noahdietz merged commit bb078a5 into googleapis:master May 15, 2020
@noahdietz noahdietz deleted the proto3-optional branch May 15, 2020 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants