Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Support for the google wrappers #49

Merged
merged 3 commits into from
Sep 11, 2023
Merged

Support for the google wrappers #49

merged 3 commits into from
Sep 11, 2023

Conversation

olegbespalov
Copy link
Collaborator

@olegbespalov olegbespalov commented Sep 7, 2023 •
edited

What?

This change allows the use of the Google Protobuff wrappers and brings the tests for that functionality.

Why?

Initially reported in grafana/k6#3232 and attempted to fix in grafana/k6#3238

@olegbespalov olegbespalov mentioned this pull request Sep 7, 2023
Closed
5 tasks
@olegbespalov olegbespalov changed the title WIP: Fix for the google wrappers support in unary calls and tests for it WIP: Fix for the google wrappers support and tests for it Sep 7, 2023
@olegbespalov olegbespalov marked this pull request as ready for review September 7, 2023 12:45
@olegbespalov olegbespalov requested a review from a team as a code owner September 7, 2023 12:45
@olegbespalov olegbespalov requested review from oleiade, codebien and mstoykov and removed request for a team and oleiade September 7, 2023 12:45
@olegbespalov olegbespalov changed the title WIP: Fix for the google wrappers support and tests for it Support for the google wrappers Sep 7, 2023
@mstoykov
Copy link
Contributor

mstoykov commented Sep 8, 2023

LGTM otherwise then us having what seems like hte same code in two places

mstoykov
mstoykov approved these changes Sep 11, 2023
View reviewed changes
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

LGTM 👍

There is a small comment problem :(

lib/netext/grpcext/stream.go Outdated Show resolved Hide resolved
codebien
codebien reviewed Sep 11, 2023
View reviewed changes
Copy link
Contributor

@codebien codebien left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for the rebase + nit fixes for final approval

@olegbespalov olegbespalov self-assigned this Sep 11, 2023
@olegbespalov
Copy link
Collaborator Author

For the record, I just rebased and squashed some commits, going to merge

@olegbespalov olegbespalov merged commit 0250ce4 into main Sep 11, 2023
10 checks passed
@olegbespalov olegbespalov deleted the fix/wrappers branch September 11, 2023 13:19
@olegbespalov olegbespalov mentioned this pull request Sep 20, 2023
Merged
5 tasks
@olegbespalov olegbespalov mentioned this pull request Oct 20, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mstoykov mstoykov mstoykov approved these changes

@codebien codebien codebien approved these changes

Assignees

@olegbespalov olegbespalov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@olegbespalov @mstoykov @codebien