Skip to content

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

Merged
alexander-fenster merged 2 commits intoprotobufjs:masterfrom
postmanlabs:fix/nested-messages-within-groups
Aug 17, 2022
Merged

fix: support for nested messages and enums within group blocks#1790
alexander-fenster merged 2 commits intoprotobufjs:masterfrom
postmanlabs:fix/nested-messages-within-groups

Conversation

@JoshuaWise
Copy link
Copy Markdown
Contributor

@JoshuaWise JoshuaWise commented Aug 11, 2022

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;
  }
}

@JoshuaWise JoshuaWise mentioned this pull request Aug 11, 2022
Closed
@alexander-fenster
Copy link
Copy Markdown
Contributor

alexander-fenster commented Aug 17, 2022

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
Copy Markdown

codenirvana commented Aug 17, 2022

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
Copy Markdown
Contributor

alexander-fenster commented Aug 17, 2022

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
Copy Markdown

codenirvana commented Aug 17, 2022

Awesome!

Also, tests passed. 🎉

@alexander-fenster alexander-fenster merged commit f36d4e4 into protobufjs:master Aug 17, 2022
@github-actions github-actions Bot mentioned this pull request Aug 17, 2022
Merged
@codenirvana codenirvana deleted the fix/nested-messages-within-groups branch August 18, 2022 02:38
@moul-bot moul-bot mentioned this pull request Sep 12, 2022
Closed
@moul-bot moul-bot mentioned this pull request Sep 23, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-fenster alexander-fenster alexander-fenster approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JoshuaWise @alexander-fenster @codenirvana @JoshuaWisePostmanaut