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: split EnvoyEngine.h into multiple header files #24397

Merged
merged 4 commits into from
Dec 7, 2022

Conversation

jpsim
Copy link
Contributor

@jpsim jpsim commented Dec 6, 2022

Resolves envoyproxy/envoy-mobile#333

Commit Message:
Additional Description:
Risk Level: Low, issues should be caught by CI, no risk to non-Objective-C/Swift targets
Testing:
Docs Changes:
Release Notes:
Platform Specific Features: Only affects Objective-C/Swift targets
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #24397 was opened by jpsim.

see: more, trace.

@repokitteh-read-only
Copy link

CC @envoyproxy/mobile-maintainers: FYI only for changes made to (mobile/).
envoyproxy/mobile-maintainers assignee is @alyssawilk

🐱

Caused by: #24397 was opened by jpsim.

see: more, trace.

Signed-off-by: JP Simard <jp@jpsim.com>
@jpsim jpsim marked this pull request as ready for review December 6, 2022 20:34
@jpsim
Copy link
Contributor Author

jpsim commented Dec 6, 2022

/assign @Augustyniak

@alyssawilk alyssawilk removed their assignment Dec 6, 2022
Augustyniak
Augustyniak previously approved these changes Dec 7, 2022
Copy link
Contributor

@Augustyniak Augustyniak left a comment

Choose a reason for hiding this comment

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

one optional comment


@property (nonatomic, copy, nonnull) void (^track)(EnvoyEvent *);

- (instancetype)initWithEventTrackingClosure:(nonnull void (^)(EnvoyEvent *))track;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit picking: not sure whether nonnull attributes in here and at line #9 are needed but it was probably just copied-over and it is not a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eye! Removed.

….h-into-multiple-header-files

* origin/main:
  Add setRequestDecoder to ResponseEncoder interface (#24368)
  downstream: refactoring code to remove listener hard deps (#24394)
  lb api: moving load balancing policy specific configuration to extension configuration (#23967)
  ci: Skip docker/examples verification for docs or mobile only changes (#24417)
  ci: run mobile GitHub Actions on every PR (#24407)
  mobile: remove `bump_lyft_support_rotation.sh` script (#24404)
  Add file size to DirectoryEntry (#24176)
  bazel: update to 6.0.0rc4 (#24235)
  bazel: update rules_rust (#24409)
  Ecds config dump recommit (#24384)
  bazel: add another config_setting incompatible flag (#24270)
  listeners: moving listeners to extension directory (#24248)
  mobile: build Swift with whole module optimization (#24396)
  ci: update `actions/setup-java` from v1 to v3.8 (#24393)

Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
@jpsim jpsim merged commit ff9ab2b into main Dec 7, 2022
@jpsim jpsim deleted the ios-split-envoyengine.h-into-multiple-header-files branch December 7, 2022 21:33
jpsim added a commit that referenced this pull request Dec 8, 2022
…-cpp-to-latest-version

* origin/main: (23 commits)
  Reduce Route memory utilization by avoiding RuntimeData instances when not needed (#24327)
  build: fix compile error for mac (#24429)
  postgres: support for upstream SSL (#23990)
  iOS: split `EnvoyEngine.h` into multiple header files (#24397)
  mobile: check for pending exceptions after JNI call (#24361)
  Remove uneccessary `this->` from mobile engine builder (#24389)
  Add setRequestDecoder to ResponseEncoder interface (#24368)
  downstream: refactoring code to remove listener hard deps (#24394)
  lb api: moving load balancing policy specific configuration to extension configuration (#23967)
  ci: Skip docker/examples verification for docs or mobile only changes (#24417)
  ci: run mobile GitHub Actions on every PR (#24407)
  mobile: remove `bump_lyft_support_rotation.sh` script (#24404)
  Add file size to DirectoryEntry (#24176)
  bazel: update to 6.0.0rc4 (#24235)
  bazel: update rules_rust (#24409)
  Ecds config dump recommit (#24384)
  bazel: add another config_setting incompatible flag (#24270)
  listeners: moving listeners to extension directory (#24248)
  mobile: build Swift with whole module optimization (#24396)
  ci: update `actions/setup-java` from v1 to v3.8 (#24393)
  ...

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build: objective-c dependencies limited to single header
3 participants