Skip to content

Remove uneccessary this-> from mobile engine builder#24389

Merged
jpsim merged 2 commits intoenvoyproxy:mainfrom
caschoener:builder_cleanup
Dec 7, 2022
Merged

Remove uneccessary this-> from mobile engine builder#24389
jpsim merged 2 commits intoenvoyproxy:mainfrom
caschoener:builder_cleanup

Conversation

@caschoener
Copy link
Copy Markdown
Contributor

This file uses this-> whenever referencing class members but it doesn't need to. Let's just ctrl-f and delete them all.

Unit tests still work!

Signed-off-by: caschoener <schoener@google.com>
Signed-off-by: caschoener <schoener@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: #24389 was opened by caschoener.

see: more, trace.

@RyanTheOptimist
Copy link
Copy Markdown
Contributor

Why did you close #24385 and open a new PR with basically the same changes?

@caschoener
Copy link
Copy Markdown
Contributor Author

That one wasn't on a branch, it was on main. So I think I had to close it?

My ssh agent was only working 50% of the time locally so some commands got lost.

Copy link
Copy Markdown
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

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

Thanks for doing this twice!

@caschoener
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24389 (comment) was created by @caschoener.

see: more, trace.

@caschoener
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24389 (comment) was created by @caschoener.

see: more, trace.

Copy link
Copy Markdown
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup!

Do you know if there's a clang-format config option for this?

@jpsim jpsim merged commit d99cd73 into envoyproxy:main Dec 7, 2022
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>
@caschoener
Copy link
Copy Markdown
Contributor Author

Do you know if there's a clang-format config option for this?

After skimming clang format docs I don't see anything, also the google3 style guide doesn't seem to mention it.

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.

4 participants