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

rlqs: address nits from PR #37797 #37851

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

bsurber
Copy link
Contributor

@bsurber bsurber commented Jan 2, 2025

Commit Message:

  • Nit 1: Lambda capture for the RLQS filter factory callback should move shared_ptrs instead of copying. This only saves a shared_ptr copy op per var but is still good practice.
  • Nit 2: Replace instances of if-conditions (!ptr) / (ptr) with (ptr == nullptr) / (ptr != nullptr) to conform to existing code.

Risk Level: minimal
Testing: unit testing
Docs Changes:
Release Notes:
Platform Specific Features:
Fixes commit #37797

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: #37851 was opened by bsurber.

see: more, trace.

Signed-off-by: Brian Surber <bsurber@google.com>
@bsurber bsurber force-pushed the fix-matcher-build-nits branch from 2693915 to f521fc8 Compare January 2, 2025 18:59
@bsurber bsurber marked this pull request as ready for review January 2, 2025 19:33
Copy link
Member

@tyxia tyxia left a comment

Choose a reason for hiding this comment

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

LGTM Thank you!

@tyxia tyxia merged commit 8e6f6a1 into envoyproxy:main Jan 6, 2025
24 checks passed
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.

2 participants