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

removing values.Map from BaseMessage proto as Beholder only supports root level protos #876

Merged
merged 4 commits into from
Oct 22, 2024
Merged

removing values.Map from BaseMessage proto as Beholder only supports root level protos #876

merged 4 commits into from
Oct 22, 2024

Conversation

patrickhuie19
Copy link
Contributor

@patrickhuie19 patrickhuie19 commented Oct 22, 2024 •
edited by jmank88
Loading

Temporary workaround while we are blocked on INFOPLAT-1386

Originally tried to embed values.Map into the proto, but it was very messy. This assumes that all labels will be strings, which is true for the short term.

Supports:

@patrickhuie19 patrickhuie19 requested review from a team as code owners October 22, 2024 18:30
@patrickhuie19 patrickhuie19 requested review from vyzaldysanchez and removed request for a team October 22, 2024 18:30
@patrickhuie19 patrickhuie19 force-pushed the feature/root-level-proto branch from c20288c to 51d04f5 Compare October 22, 2024 18:32
@patrickhuie19 patrickhuie19 force-pushed the feature/root-level-proto branch from 51d04f5 to bd049e6 Compare October 22, 2024 18:36
vyzaldysanchez
vyzaldysanchez previously requested changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@vyzaldysanchez vyzaldysanchez left a comment

Choose a reason for hiding this comment

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

LGTM, but we need CI working.

@patrickhuie19 patrickhuie19 mentioned this pull request Oct 22, 2024
Closed
@patrickhuie19 patrickhuie19 merged commit b772997 into main Oct 22, 2024
10 of 11 checks passed
@patrickhuie19 patrickhuie19 deleted the feature/root-level-proto branch October 22, 2024 20:55
patrickhuie19 added a commit that referenced this pull request Oct 23, 2024
@patrickhuie19
Revert "removing values.Map from BaseMessage proto as Beholder only s…
a957aee 
…upports root level protos (#876)"

This reverts commit b772997.
vyzaldysanchez pushed a commit that referenced this pull request Oct 23, 2024
@patrickhuie19
Revert "removing values.Map from BaseMessage proto as Beholder only s…
c5c856e 
…upports root level protos (#876)" (#881)

This reverts commit b772997.
patrickhuie19 added a commit that referenced this pull request Oct 23, 2024
@patrickhuie19
Reapply "removing values.Map from BaseMessage proto as Beholder only …
59b68ff 
…supports root level protos (#876)" (#881)

This reverts commit c5c856e.
patrickhuie19 added a commit that referenced this pull request Oct 25, 2024
@patrickhuie19
Reapply "removing values.Map from BaseMessage proto as Beholder only …
eed6b5f 
…supports root level protos (#876)" (#881) (#886)

This reverts commit c5c856e.
patrickhuie19 added a commit that referenced this pull request Oct 25, 2024
@patrickhuie19
Reapply "removing values.Map from BaseMessage proto as Beholder only …
455f42f 
…supports root level protos (#876)" (#881) (#886)

This reverts commit c5c856e.

(cherry picked from commit eed6b5f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krehermann krehermann krehermann approved these changes

@vyzaldysanchez vyzaldysanchez vyzaldysanchez 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.
3 participants
@patrickhuie19 @vyzaldysanchez @krehermann