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

fix: support for nested messages and enums within group blocks #1790

Conversation

JoshuaWise
Copy link
Contributor

This fixes support for proto files that have nested messages or enums declared within groups, like this:

syntax = "proto2";

message Foo {
  repeated group Field = 1 {
    message Bar {
      optional bool my_bool = 2;
    }

    optional string my_string = 3;
    optional Bar my_bar = 4;
  }
}

@alexander-fenster
Copy link
Contributor

Hi @JoshuaWise, could you give me access to your branch (there should be a checkbox that allows contributors to edit the code) so I could run the CI and fix failures if there are any? A little bit scared to merge this without seeing the tests pass.

Alternatively, you might be able to update your fork to point to the latest head of the repository, which will hopefully make GitHub run tests for this PR.

@codenirvana codenirvana force-pushed the fix/nested-messages-within-groups branch from a75d7b8 to bffd4cf Compare August 17, 2022 17:53
@codenirvana
Copy link

Hey @alexander-fenster, Josh is unavailable this month. I am the maintainer of the fork.
I tried rebasing the branch with protobufjs:master but looks like that didn’t help. 😞

@alexander-fenster
Copy link
Contributor

It did help! I was able to run tests now (I needed to approve the workflow by hitting a button, but the button just did not exist before you update the branch). Let's see what happens!

@codenirvana
Copy link

Awesome!

Also, tests passed. 🎉

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.

None yet

4 participants