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.

Sign up for GitHub

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

Jump to bottom

Generate clear_foo methods for protobuf compiler #1284

Merged
merged 1 commit into from
Nov 29, 2022
Merged

Generate clear_foo methods for protobuf compiler #1284

merged 1 commit into from
Nov 29, 2022

Conversation

slinkp
Copy link
Contributor

@slinkp slinkp commented Nov 28, 2022

Protobuf: Add support for clear_* methods

Motivation

Per the docs, Protobuf generates a method for every field that clears it. We should have sigs declared for those.

Implementation

Just added to the existing create_descriptor_method

Tests

Yep

Generate clear_foo methods for protobuf compiler
583e2a9 
Protobuf: Add support for clear_* fields
@slinkp slinkp requested a review from a team as a code owner November 28, 2022 23:25
@slinkp slinkp self-assigned this Nov 28, 2022
andrewn617
andrewn617 approved these changes Nov 29, 2022
View reviewed changes
Copy link
Contributor

@andrewn617 andrewn617 left a comment

Choose a reason for hiding this comment

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

LGTM!

dirceu
dirceu approved these changes Nov 29, 2022
View reviewed changes
Copy link
Contributor

@dirceu dirceu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just tested this against Shopify's core monolith and it looks good! Lots of new RBI declarations and no new typechecking errors. Thank you for your contribution!

@dirceu dirceu merged commit 64c09e8 into main Nov 29, 2022
@dirceu dirceu deleted the add_protobuf_clear_method branch November 29, 2022 17:48
@paracycle paracycle added the enhancement New feature or request label Dec 19, 2022
@shopify-shipit shopify-shipit bot temporarily deployed to production December 19, 2022 22:14 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dirceu dirceu dirceu approved these changes

@KaanOzkan KaanOzkan KaanOzkan approved these changes

@andrewn617 andrewn617 andrewn617 approved these changes

Assignees

@slinkp slinkp

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@slinkp @dirceu @KaanOzkan @andrewn617 @paracycle