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

ios: utilize engine handle at platform layer #327

Closed
wants to merge 3 commits into from

Conversation

goaway
Copy link
Contributor

@goaway goaway commented Aug 14, 2019

Signed-off-by: Mike Schore mike.schore@gmail.com

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
}

- (instancetype)initWithEngine:(envoy_engine_t)engine observer:(EnvoyObserver *)observer {
- (instancetype)initWithHandle:(envoy_stream_t)streamHandle observer:(EnvoyObserver *)observer {

Choose a reason for hiding this comment

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

I actually like this approach more than the original engine approach

This is a similar concept to what I was thinking about doing for the java side

Definitely can chat more tomorrow in case we think of a better solution

rebello95
rebello95 previously approved these changes Aug 14, 2019
buildbreaker
buildbreaker previously approved these changes Aug 14, 2019
Copy link

@buildbreaker buildbreaker left a comment

Choose a reason for hiding this comment

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

Let's go with this for now and we can circle back if/when we think we have something better @goaway

We'll get another opportunity when we introduce the engine handler 😄

Signed-off-by: Mike Schore <mike.schore@gmail.com>
@goaway goaway dismissed stale reviews from buildbreaker and rebello95 via 573045b August 14, 2019 18:14
@goaway
Copy link
Contributor Author

goaway commented Aug 14, 2019

recreated here: #329

@goaway goaway closed this Aug 14, 2019
@goaway goaway deleted the ms/stream-creation branch August 16, 2019 16:56
buildbreaker pushed a commit that referenced this pull request Aug 16, 2019
Updating java library layer to conform to the streaming interfaces changes. Also, tweaks to make the java layer more consistent with the ObjC layer in #327

Signed-off-by: Alan Chiu <achiu@lyft.com>

For an explanation of how to fill out the fields, please see the relevant section
in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md)

Description: Updating java library interfaces
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]
jpsim pushed a commit to envoyproxy/envoy that referenced this pull request Nov 28, 2022
Updating java library layer to conform to the streaming interfaces changes. Also, tweaks to make the java layer more consistent with the ObjC layer in envoyproxy/envoy-mobile#327

Signed-off-by: Alan Chiu <achiu@lyft.com>

For an explanation of how to fill out the fields, please see the relevant section
in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md)

Description: Updating java library interfaces
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit to envoyproxy/envoy that referenced this pull request Nov 29, 2022
Updating java library layer to conform to the streaming interfaces changes. Also, tweaks to make the java layer more consistent with the ObjC layer in envoyproxy/envoy-mobile#327

Signed-off-by: Alan Chiu <achiu@lyft.com>

For an explanation of how to fill out the fields, please see the relevant section
in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md)

Description: Updating java library interfaces
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: JP Simard <jp@jpsim.com>
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.

3 participants