-
Notifications
You must be signed in to change notification settings - Fork 123
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
fix: annotating some fields as REQUIRED #1695
Conversation
These fields were actually always required by the backend, so annotation just documents status quo. I believe this change will not require major version bump for any language. PiperOrigin-RevId: 429093810 Source-Link: googleapis/googleapis@dc04c1c Source-Link: https://github.com/googleapis/googleapis-gen/commit/0e23469bea2f397f2b783c5a25e64452f86be6bc Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMGUyMzQ2OWJlYTJmMzk3ZjJiNzgzYzVhMjVlNjQ0NTJmODZiZTZiYyJ9
Is this a breaking change? If so, it would require us to do a major release, which is not ideal @ansh0l could you follow up? |
Sure @thiagotnunes, will followup |
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
@ansh0l cl/429093810 has description about the change and it says the author doesn't expect breaking change in customers' code. If this is the case, there's no need to bump the major version. The automation bot reads "!" in "feat!" for breaking changes. If this is not a breaking change, then remove "!" from the PR title. When merging, ensure the squashed commit to the main branch doesn't have "!" either. |
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.
LGTM, will go with suggestion by @suztomo for minor revision bump
Will be parking this PR for some more time before other major PRs are shipped for java and jdbc. |
@ansh0l afaik we can merge now, because we removed the |
@thiagotnunes I think we are good. I was busy with Spangres merges yesterday, so kept it waiting for another day. |
* chore(java): update gcp-releasetool and cryptography in java requirements file Source-Link: googleapis/synthtool@74d0956 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:142286d973c7b6d58186070f203b50058a20a7d7b42147996db24921a18da1b0
…s#1695) (googleapis#1043) * chore(java): update gcp-releasetool and cryptography in java requirements file Source-Link: https://togithub.com/googleapis/synthtool/commit/74d0956884c1bb9dc901b52de35ca2bca025a74e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:142286d973c7b6d58186070f203b50058a20a7d7b42147996db24921a18da1b0
These fields were actually always required by the backend, so annotation just documents status quo. I believe this change will not require major version bump for any language.
PiperOrigin-RevId: 429093810
Source-Link: googleapis/googleapis@dc04c1c
Source-Link: https://github.com/googleapis/googleapis-gen/commit/0e23469bea2f397f2b783c5a25e64452f86be6bc
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMGUyMzQ2OWJlYTJmMzk3ZjJiNzgzYzVhMjVlNjQ0NTJmODZiZTZiYyJ9