Conversation
Contributor
📝 WalkthroughWalkthroughPinned Python to 3.8 in CI, commented-out Windows entries in CI matrices, disabled two analyzer jobs by forcing their conditions false, added Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / PR
participant GH as GitHub Actions
participant Conan as Conan (RppConan)
participant CMake as CMakePresets
Dev->>GH: push PR / update workflows & files
GH->>GH: evaluate workflow matrices
Note right of GH #F8F0F0: Windows entries commented out\nCertain analyzer jobs `if` conditions forced false
GH->>GH: run `cache_deps` job
GH->>GH: setup-python with python-version: "3.8"
GH->>Conan: run `conan configure`
Note right of Conan #F0FFF0: configure() disables grpc plugins/options\nand uses upgraded grpc
Conan-->>GH: configuration completed
GH->>CMake: read CMakePresets.json during build
Note right of CMake #FFF7E6: flags-unix adds clang warning flag\n`RPP_BUILD_SFML_CODE` = OFF
CMake-->>GH: build proceeds
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (2 warnings, 1 inconclusive)❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci v2.yml (1)
39-41: Bump Python version to 3.10 in CI workflows
Align bothactions/setup-python@v5steps to use a non-EOL Python (3.10) to avoid future breakage..github/workflows/ci v2.yml @@ -39,2 +39,2 - - uses: actions/setup-python@v5 - with: { python-version: "3.8" } + - uses: actions/setup-python@v5 + with: { python-version: "3.10" } @@ -226,2 +226,2 - - uses: actions/setup-python@v5 - with: { python-version: "3.8" } + - uses: actions/setup-python@v5 + with: { python-version: "3.10" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci v2.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Cache deps on ci-ubuntu-gcc for Release
- GitHub Check: Cache deps on ci-macos for Release
- GitHub Check: Cache deps on ci-macos for Debug
- GitHub Check: Cache deps on ci-windows for Debug
- GitHub Check: Cache deps on ci-windows for Release
- GitHub Check: Cache deps on ci-ubuntu-clang for Debug
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci v2.yml (2)
42-46: Ensure pip targets the setup-python interpreter and consider pinning versions for reproducibility.
- Use
python -m pipto bind installs to theactions/setup-pythonruntime.- Optional: pin exact versions in a requirements file to avoid surprise CI changes.
Apply:
- run: | - sudo apt-get update -q - pip install pyyaml jinja2 + run: | + sudo apt-get update -q + python -m pip install --upgrade pip + python -m pip install pyyaml jinja2
40-41: Use a supported Python version (3.10+) in CI. Replace bothpython-version: "3.8"entries in .github/workflows/ci v2.yml (lines 40 and 233) withpython-version: "3.10"(and consider addingcache: pip).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci v2.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cache deps on ci-ubuntu-clang for Debug
- GitHub Check: Cache deps on ci-ubuntu-gcc for Release
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci v2.yml (1)
40-41: Pinning to Python 3.8 (EOL) — verify necessity and align across jobs.3.8 reached EOL; prefer 3.10+ unless a hard constraint exists. If you keep 3.8, ensure all jobs that use Python install deps into that same interpreter (currently tests use pip without setup-python).
Optional improvement:
- - uses: actions/setup-python@v5 - with: { python-version: "3.8" } + - uses: actions/setup-python@v5 + with: { python-version: "3.10", cache: "pip" }If 3.8 is required, consider adding
cache: "pip"here too and updating other jobs to install viapython -m pipfor consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci v2.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cache deps on ci-ubuntu-clang for Debug
- GitHub Check: Cache deps on ci-ubuntu-gcc for Release
Comment on lines
+45
to
+51
| - name: Install deps | ||
| if: matrix.config.os == 'ubuntu-22.04' | ||
| run: | | ||
| sudo apt-get update -q | ||
| pip3 install pyyaml jinja2 | ||
| sudo apt-get install python3-jinja2 | ||
|
|
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix Python env mismatch and non-interactive install.
- pip3 and apt install to the system Python, not the 3.8 set by actions/setup-python, which can cause ModuleNotFoundError at runtime.
- Avoid double-installing Jinja2 (pip + apt). Also, apt install is missing -y and may hang.
Apply:
- - name: Install deps
- if: matrix.config.os == 'ubuntu-22.04'
- run: |
- sudo apt-get update -q
- pip3 install pyyaml jinja2
- sudo apt-get install python3-jinja2
+ - name: Install deps
+ if: matrix.config.os == 'ubuntu-22.04'
+ run: |
+ sudo apt-get update -q
+ python -m pip install --upgrade pip
+ python -m pip install pyyaml jinja2Optionally mirror this step on macOS/Windows if these deps are needed in those matrix jobs too.
🏁 Script executed:
#!/bin/bash
# Locate usage of actions/setup-python in the CI workflow
grep -R "actions/setup-python" -n .github/workflows/
echo "---- First 100 lines of .github/workflows/ci v2.yml ----"
sed -n '1,100p' ".github/workflows/ci v2.yml"
echo "---- Lines 40-60 of .github/workflows/ci v2.yml ----"
sed -n '40,60p' ".github/workflows/ci v2.yml"Length of output: 4022
Fix Python package installs in CI
- Drop the system
apt-get install python3-jinja2(its packages go to the host Python, not the 3.8 from actions/setup-python). - Use
python -m pip install(and upgrade pip) so dependencies land in the configured Python environment.
- pip3 install pyyaml jinja2
- sudo apt-get install python3-jinja2
+ python -m pip install --upgrade pip
+ python -m pip install pyyaml jinja2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Install deps | |
| if: matrix.config.os == 'ubuntu-22.04' | |
| run: | | |
| sudo apt-get update -q | |
| pip3 install pyyaml jinja2 | |
| sudo apt-get install python3-jinja2 | |
| - name: Install deps | |
| if: matrix.config.os == 'ubuntu-22.04' | |
| run: | | |
| sudo apt-get update -q | |
| python -m pip install --upgrade pip | |
| python -m pip install pyyaml jinja2 |
🤖 Prompt for AI Agents
.github/workflows/ci v2.yml around lines 45 to 51: the workflow currently
installs Python packages via pip3 and a system apt package (python3-jinja2)
which installs into the system Python rather than the actions/setup-python
virtual environment; remove the apt-get install python3-jinja2 line, replace
pip3 install with python -m pip install after running python -m pip install
--upgrade pip to ensure packages are installed into the configured Python (the
same python provided by actions/setup-python), and keep the pip install list
(pyyaml jinja2) using that python -m pip invocation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci v2.yml (1)
45-50: Fix Python env mismatch and make apt non-interactive.pip3 + apt python3-jinja2 install into the system Python, not the actions/setup-python interpreter, causing ModuleNotFoundError. Also, apt install lacks -y and can hang. Install Python deps with the runner’s Python and keep apt minimal.
- - name: Install deps - if: matrix.config.os == 'ubuntu-22.04' - run: | - sudo apt-get update -q - pip3 install pyyaml jinja2 - sudo apt-get install libsystemd-dev python3-jinja2 + - name: Install deps + if: matrix.config.os == 'ubuntu-22.04' + run: | + sudo apt-get update -q + sudo apt-get install -y -q libsystemd-dev + python -m pip install --upgrade pip + python -m pip install pyyaml jinja2
🧹 Nitpick comments (1)
.github/workflows/ci v2.yml (1)
39-41: Pinning to Python 3.8 (EOL) — consider upgrading to a supported version.Python 3.8 reached EOL in 2024; pinning CI to it increases risk and limits dependency resolution. Recommend 3.11 (or 3.10 if you need broader ABI compat). If you change this, mirror the version across all jobs that use Python.
- - uses: actions/setup-python@v5 - with: { python-version: "3.8" } + - uses: actions/setup-python@v5 + with: { python-version: "3.11" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci v2.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Cache deps on ci-macos for Debug
- GitHub Check: Cache deps on ci-macos for Release
- GitHub Check: Cache deps on ci-windows for Debug
- GitHub Check: Cache deps on ci-windows for Release
- GitHub Check: Cache deps on ci-ubuntu-clang for Debug
- GitHub Check: Cache deps on ci-ubuntu-gcc for Release
- GitHub Check: Build Doxygen Docs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
conanfile.py (2)
36-36: with_libsystemd=False while CI installs libsystemd-devThis looks inconsistent. Either enable the feature or drop the extra system package to keep CI lean.
26-38: Prefer passing dependency options in requires() (closer to the edge where they apply)Setting dep options in configure() works but overrides consumer intent. Co-locating them with requires() improves clarity and scoping.
Example outside this hunk (illustrative):
if self.options.with_grpc: self.requires( "grpc/1.54.3", options={ "codegen": False, # or True, per your choice above "cpp_plugin": False, # keep consistent with codegen "with_libsystemd": False, "php_plugin": False, "node_plugin": False, "ruby_plugin": False, "python_plugin": False, "csharp_plugin": False, "objective_c_plugin": False, "csharp_ext": False, "otel_plugin": False, }, transitive_libs=True, transitive_headers=True, )If you need protoc plugins only as build tools, consider tool_requires("grpc/...") with codegen/plugin enabled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
conanfile.py(1 hunks)
🔇 Additional comments (2)
conanfile.py (2)
27-37: Verify grpc option names in Conan recipe
Confirm that all options set underself.options["grpc/*"](e.g.,objective_c_plugin,with_libsystemd,otel_plugin,csharp_ext) actually exist in thegrpc/1.54.3recipe on ConanCenter to avoid graph resolution failures; if any are missing, either guard their assignment with a check or remove them.
27-28: Align grpc codegen and C++ plugin options
Either enable codegen when using the C++ plugin or disable the plugin when skipping codegen:
- To build the C++ protoc plugin, set
self.options["grpc/*"].codegen = True- To omit codegen tools, set
self.options["grpc/*"].cpp_plugin = False
Confirm which intent should be reflected in CI.
Comment on lines
+26
to
+38
| def configure(self): | ||
| self.options["grpc/*"].codegen = False | ||
| self.options["grpc/*"].cpp_plugin = True | ||
| self.options["grpc/*"].csharp_ext = False | ||
| self.options["grpc/*"].php_plugin = False | ||
| self.options["grpc/*"].node_plugin = False | ||
| self.options["grpc/*"].otel_plugin = False | ||
| self.options["grpc/*"].ruby_plugin = False | ||
| self.options["grpc/*"].csharp_plugin = False | ||
| self.options["grpc/*"].python_plugin = False | ||
| self.options["grpc/*"].with_libsystemd = False | ||
| self.options["grpc/*"].objective_c_plugin = False | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate gRPC option overrides behind with_grpc to avoid global graph side-effects
Unconditionally overriding dependency options can leak into consumer graphs and fail builds even when gRPC is off. Guard these settings.
Apply this diff:
def configure(self):
- self.options["grpc/*"].codegen = False
- self.options["grpc/*"].cpp_plugin = True
- self.options["grpc/*"].csharp_ext = False
- self.options["grpc/*"].php_plugin = False
- self.options["grpc/*"].node_plugin = False
- self.options["grpc/*"].otel_plugin = False
- self.options["grpc/*"].ruby_plugin = False
- self.options["grpc/*"].csharp_plugin = False
- self.options["grpc/*"].python_plugin = False
- self.options["grpc/*"].with_libsystemd = False
- self.options["grpc/*"].objective_c_plugin = False
+ if not self.options.with_grpc:
+ return
+ self.options["grpc/*"].codegen = False
+ self.options["grpc/*"].cpp_plugin = True
+ self.options["grpc/*"].csharp_ext = False
+ self.options["grpc/*"].php_plugin = False
+ self.options["grpc/*"].node_plugin = False
+ self.options["grpc/*"].otel_plugin = False
+ self.options["grpc/*"].ruby_plugin = False
+ self.options["grpc/*"].csharp_plugin = False
+ self.options["grpc/*"].python_plugin = False
+ self.options["grpc/*"].with_libsystemd = False
+ self.options["grpc/*"].objective_c_plugin = False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def configure(self): | |
| self.options["grpc/*"].codegen = False | |
| self.options["grpc/*"].cpp_plugin = True | |
| self.options["grpc/*"].csharp_ext = False | |
| self.options["grpc/*"].php_plugin = False | |
| self.options["grpc/*"].node_plugin = False | |
| self.options["grpc/*"].otel_plugin = False | |
| self.options["grpc/*"].ruby_plugin = False | |
| self.options["grpc/*"].csharp_plugin = False | |
| self.options["grpc/*"].python_plugin = False | |
| self.options["grpc/*"].with_libsystemd = False | |
| self.options["grpc/*"].objective_c_plugin = False | |
| def configure(self): | |
| # Only override grpc/* options when gRPC support is explicitly enabled | |
| if not self.options.with_grpc: | |
| return | |
| self.options["grpc/*"].codegen = False | |
| self.options["grpc/*"].cpp_plugin = True | |
| self.options["grpc/*"].csharp_ext = False | |
| self.options["grpc/*"].php_plugin = False | |
| self.options["grpc/*"].node_plugin = False | |
| self.options["grpc/*"].otel_plugin = False | |
| self.options["grpc/*"].ruby_plugin = False | |
| self.options["grpc/*"].csharp_plugin = False | |
| self.options["grpc/*"].python_plugin = False | |
| self.options["grpc/*"].with_libsystemd = False | |
| self.options["grpc/*"].objective_c_plugin = False |
🤖 Prompt for AI Agents
In conanfile.py around lines 26 to 38, the code unconditionally overrides
options for "grpc/*" which can leak into consumer graphs; wrap those assignments
in a guard that only runs when the package's with_grpc option is enabled. Change
the block to check the package option (e.g. if getattr(self.options,
"with_grpc", False) or if self.options.get_safe("with_grpc")) and only then set
self.options["grpc/*"].codegen = False etc.; leave the assignments out when
with_grpc is False so they don't affect unrelated dependency graphs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CMakePresets.json (1)
41-41: Scope the new suppression and propagate it to sanitize presetsGood quick fix to unblock CI. Two follow-ups:
- Limit this to compilers that actually emit the diagnostic (currently Clang), and
- Ensure sanitize builds don’t drop the suppression by overriding CMAKE_CXX_FLAGS.
Minimal change inside this file: append the same suppression (and keep -Wno-unknown-warning-option for Clang) to the sanitize presets’ CMAKE_CXX_FLAGS:
@@ "name": "ci-sanitize-tsan", @@ - "CMAKE_CXX_FLAGS": "-fsanitize=thread -g -O1" + "CMAKE_CXX_FLAGS": "-fsanitize=thread -g -O1 -Wno-missing-template-arg-list-after-template-kw -Wno-unknown-warning-option" @@ "name": "ci-sanitize-asan", @@ - "CMAKE_CXX_FLAGS": "-fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1" + "CMAKE_CXX_FLAGS": "-fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1 -Wno-missing-template-arg-list-after-template-kw -Wno-unknown-warning-option" @@ "name": "ci-sanitize-lsan", @@ - "CMAKE_CXX_FLAGS": "-fsanitize=leak -fno-omit-frame-pointer -g -O1" + "CMAKE_CXX_FLAGS": "-fsanitize=leak -fno-omit-frame-pointer -g -O1 -Wno-missing-template-arg-list-after-template-kw -Wno-unknown-warning-option" @@ "name": "ci-sanitize-msan", @@ - "CMAKE_CXX_FLAGS": "-fsanitize=memory -fno-optimize-sibling-calls -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g -O2" + "CMAKE_CXX_FLAGS": "-fsanitize=memory -fno-optimize-sibling-calls -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g -O2 -Wno-missing-template-arg-list-after-template-kw -Wno-unknown-warning-option" @@ "name": "ci-sanitize-ubsan", @@ - "CMAKE_CXX_FLAGS": "-fsanitize=undefined" + "CMAKE_CXX_FLAGS": "-fsanitize=undefined -Wno-missing-template-arg-list-after-template-kw -Wno-unknown-warning-option"Longer-term, consider moving warnings to target_compile_options() with generator expressions per compiler/version to avoid clobbering across presets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
CMakePresets.json(1 hunks)conanfile.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- conanfile.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Cache deps on ci-macos for Debug
- GitHub Check: Cache deps on ci-macos for Release
- GitHub Check: Cache deps on ci-windows for Debug
- GitHub Check: Cache deps on ci-windows for Release
- GitHub Check: Cache deps on ci-ubuntu-gcc for Debug
- GitHub Check: Cache deps on ci-ubuntu-clang for Debug
- GitHub Check: Cache deps on ci-ubuntu-clang for Release
- GitHub Check: Cache deps on ci-ubuntu-gcc for Release
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
conanfile.py (1)
26-34: Gate gRPC option overrides behind with_grpc to avoid leaking settings into consumer graphsUnconditionally overriding
"grpc/*"options affects builds even whenwith_grpc=False. Guard these assignments.def configure(self): - self.options["grpc/*"].with_libsystemd = False - self.options["grpc/*"].csharp_plugin = False - self.options["grpc/*"].node_plugin = False - self.options["grpc/*"].objective_c_plugin = False - self.options["grpc/*"].php_plugin = False - self.options["grpc/*"].python_plugin = False - self.options["grpc/*"].ruby_plugin = False + if not self.options.get_safe("with_grpc"): + return + self.options["grpc/*"].with_libsystemd = False + self.options["grpc/*"].csharp_plugin = False + self.options["grpc/*"].node_plugin = False + self.options["grpc/*"].objective_c_plugin = False + self.options["grpc/*"].php_plugin = False + self.options["grpc/*"].python_plugin = False + self.options["grpc/*"].ruby_plugin = False
🧹 Nitpick comments (1)
conanfile.py (1)
54-56: Confirm CMake pin and consider moving to profilesPinning
cmake/3.29.3can break CI runners lacking this exact revision. Verify availability across your CI and caches; consider moving the requirement into the profile or using a compatible range if feasible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
conanfile.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
conanfile.py
58-58: Undefined name CMakeDeps
(F821)
60-60: Undefined name CMakeToolchain
(F821)
Comment on lines
+57
to
+64
| def generate(self): | ||
| deps = CMakeDeps(self) | ||
| deps.generate() | ||
| tc = CMakeToolchain(self) | ||
|
|
||
| if self.settings.compiler == "clang": | ||
| tc.extra_cxxflags.extend(['-Wno-missing-template-arg-list-after-template-kw']) | ||
| tc.generate() |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix NameError for CMakeDeps/CMakeToolchain and avoid double-generation
- Missing imports cause
NameError(Ruff F821). Add the import. - Since you manually generate CMakeDeps/Toolchain, drop the
generators = "CMakeDeps", "CMakeToolchain"attribute to prevent duplicate files. - Use
get_safefor the compiler check.
Apply within this hunk:
- if self.settings.compiler == "clang":
+ if self.settings.get_safe("compiler") == "clang":
tc.extra_cxxflags.extend(['-Wno-missing-template-arg-list-after-template-kw'])And elsewhere in the file (outside this hunk):
# top-level imports
from conan.tools.cmake import CMakeDeps, CMakeToolchain
# remove the attribute to avoid double-generation
# generators = "CMakeDeps", "CMakeToolchain"🧰 Tools
🪛 Ruff (0.12.2)
58-58: Undefined name CMakeDeps
(F821)
60-60: Undefined name CMakeToolchain
(F821)
🤖 Prompt for AI Agents
In conanfile.py around lines 57 to 64, the generate() function references
CMakeDeps and CMakeToolchain causing a NameError because they are not imported,
and the class still declares generators = "CMakeDeps", "CMakeToolchain" which
leads to double-generation; add top-level imports `from conan.tools.cmake import
CMakeDeps, CMakeToolchain`, remove the `generators = "CMakeDeps",
"CMakeToolchain"` attribute from the class, and change the compiler check to use
`self.settings.compiler.get_safe()` (e.g. `if self.settings.compiler.get_safe()
== "clang":`) while keeping the manual deps.generate() and tc.generate() calls.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
conanfile.py (2)
26-34: Gate grpc option overrides behind with_grpcUnconditional overrides can leak into consumer graphs and fail builds when gRPC is off.
def configure(self): - self.options["grpc/*"].with_libsystemd = False - self.options["grpc/*"].csharp_plugin = False - self.options["grpc/*"].node_plugin = False - self.options["grpc/*"].objective_c_plugin = False - self.options["grpc/*"].php_plugin = False - self.options["grpc/*"].python_plugin = False - self.options["grpc/*"].ruby_plugin = False + if not self.options.with_grpc: + return + grpc = self.options["grpc/*"] + grpc.with_libsystemd = False + grpc.csharp_plugin = False + grpc.node_plugin = False + grpc.objective_c_plugin = False + grpc.php_plugin = False + grpc.python_plugin = False + grpc.ruby_plugin = False
57-64: ImportCMakeDeps/CMakeToolchain, remove class-level generators, and broaden compiler check
- Add at top of
conanfile.py:from conan import ConanFile +from conan.tools.cmake import CMakeDeps, CMakeToolchain- In class header, delete the line:
- generators = "CMakeDeps", "CMakeToolchain"- In
def generate(self):, change:- if self.settings.compiler == "apple-clang": + if self.settings.get_safe("compiler") in ("clang", "apple-clang"):
🧹 Nitpick comments (1)
conanfile.py (1)
47-49: Confirm availability and pin consistently across graphEnsure grpc/1.65.5 and protobuf/5.26.1 exist on your remotes; consider overriding protobuf to avoid version skew via transitive deps.
- self.requires("protobuf/5.26.1") + self.requires("protobuf/5.26.1", override=True)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
conanfile.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
conanfile.py
58-58: Undefined name CMakeDeps
(F821)
60-60: Undefined name CMakeToolchain
(F821)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
conanfile.py (1)
26-34: Gate gRPC option overrides behind with_grpc to avoid leaking global optionsUnconditional
"grpc/*"assignments can affect consumer graphs even when gRPC is off. Guard them.def configure(self): - self.options["grpc/*"].with_libsystemd = False - self.options["grpc/*"].csharp_plugin = False - self.options["grpc/*"].node_plugin = False - self.options["grpc/*"].objective_c_plugin = False - self.options["grpc/*"].php_plugin = False - self.options["grpc/*"].python_plugin = False - self.options["grpc/*"].ruby_plugin = False + if not self.options.get_safe("with_grpc"): + return + self.options["grpc/*"].with_libsystemd = False + self.options["grpc/*"].csharp_plugin = False + self.options["grpc/*"].node_plugin = False + self.options["grpc/*"].objective_c_plugin = False + self.options["grpc/*"].php_plugin = False + self.options["grpc/*"].python_plugin = False + self.options["grpc/*"].ruby_plugin = FalseRun to confirm these options still exist for grpc/1.65.0:
#!/bin/bash # List grpc options and check for the plugins used here conan inspect -r=conancenter grpc/1.65.0 -a options | tr ',{}' '\n' | rg -n 'cpp_plugin|python_plugin|ruby_plugin|php_plugin|node_plugin|objective_c_plugin|with_libsystemd'
🧹 Nitpick comments (1)
conanfile.py (1)
43-44: with_sfml is now inert; either restore the conditional require or remove the optionCurrently the option exists but does nothing. Pick one for clarity.
Option A (restore behavior):
- # if self.options.with_sfml: - # self.requires("sfml/2.6.2", options={"audio": False}) + if self.options.with_sfml: + self.requires("sfml/2.6.2", options={"audio": False})Option B (retire SFML cleanly): remove
with_sfmlfromoptionsanddefault_optionsat the top of the recipe.# Outside this hunk (top of class): # - Drop "with_sfml" from `options` and `default_options`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakePresets.json(2 hunks)conanfile.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CMakePresets.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Cache deps on ci-macos for Debug
- GitHub Check: Cache deps on ci-macos for Release
- GitHub Check: Cache deps on ci-windows for Release
- GitHub Check: Cache deps on ci-ubuntu-gcc for Debug
- GitHub Check: Cache deps on ci-windows for Debug
- GitHub Check: Cache deps on ci-ubuntu-clang for Release
- GitHub Check: Cache deps on ci-ubuntu-gcc for Release
- GitHub Check: Cache deps on ci-ubuntu-clang for Debug
- GitHub Check: Test RPP as package - ubuntu-22.04
🔇 Additional comments (1)
conanfile.py (1)
46-50: Gate libmount override to Linux and drop dead protobuf commentif self.options.with_grpc: self.requires("grpc/1.65.0", transitive_libs=True, transitive_headers=True) - # self.requires("protobuf/5.26.1") - self.requires("libmount/2.39", override=True) + if self.settings.get_safe("os") == "Linux": + self.requires("libmount/2.39", override=True)Ensure gRPC’s transitive protobuf version remains acceptable; verify via
conan graph info.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/ci v2.yml (2)
169-174: Use setup-python and install with python -m pip to avoid env mismatch.This job doesn’t configure Python, yet uses pip. On ubuntu-22.04, pip may target the system Python, not a consistent runtime. Add actions/setup-python and switch to python -m pip.
- name: ccache uses: hendrikmuhs/ccache-action@v1.2 with: key: ${{ matrix.type }}-${{ matrix.config.name }}-${{ matrix.build_type.config }} - - name: Install deps + - uses: actions/setup-python@v5 + if: matrix.config.os == 'ubuntu-22.04' + with: { python-version: "3.8" } + + - name: Install deps if: matrix.config.os == 'ubuntu-22.04' run: | sudo apt-get update -q && sudo apt-get install clang-tidy cppcheck -y -q - pip install pyyaml + python -m pip install --upgrade pip + python -m pip install pyyaml
231-236: Make docs install non-interactive and consistent with the configured Python.Use python -m pip (matches setup-python@v5) and add -y to apt to prevent hangs.
- name: Install deps run: | - pip3 install rxmarbles - sudo apt-get update && sudo apt-get install texlive-font-utils + python -m pip install --upgrade pip + python -m pip install rxmarbles + sudo apt-get update -q && sudo apt-get install -y texlive-font-utils doxygen --version
🧹 Nitpick comments (2)
.github/workflows/ci v2.yml (2)
42-44: Pin third-party actions to a tag or SHA (avoid @main).@main can introduce breaking changes into CI. Prefer a stable major tag or commit SHA.
- - name: get conan - uses: turtlebrowser/get-conan@main + - name: get conan + uses: turtlebrowser/get-conan@v1If you prefer immutable pins, I can provide the latest verified commit SHA.
Also applies to: 105-107, 184-186
39-41: Use Python 3.10+ in CI or ensure Python 3.8 archive availability
Python 3.8 was removed from GitHub-hosted runner images in June 2025; actions/setup-python@v5 will still download 3.8 from archives (with slower installs and potential future risk). Consider bumping to 3.10+ for built-in support and faster CI; if you must remain on 3.8, confirm archive access meets your requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci v2.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cache deps on ci-macos for Release
- GitHub Check: Cache deps on ci-macos for Debug
🔇 Additional comments (1)
.github/workflows/ci v2.yml (1)
265-275: Override to gcc-10/g++-10 is safe on ubuntu-22.04 – these versions are preinstalled. (github.com)Likely an incorrect or invalid review comment.
Contributor
BENCHMARK RESULTS (AUTOGENERATED)
|
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 303.23 ns | 1.87 ns | 1.87 ns | 1.00 | 1.87 ns |
| Subscribe empty callbacks to empty observable via pipe operator | 305.75 ns | 1.86 ns | 1.87 ns | 1.00 | 1.86 ns |
Sources
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 699.41 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| from array of 1 - create + subscribe + current_thread | 1055.25 ns | 3.73 ns | 3.73 ns | 1.00 | 3.73 ns |
| concat_as_source of just(1 immediate) create + subscribe | 2253.14 ns | 112.85 ns | 112.83 ns | 1.00 | 114.03 ns |
| defer from array of 1 - defer + create + subscribe + immediate | 728.05 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| interval - interval + take(3) + subscribe + immediate | 2193.10 ns | 59.60 ns | 60.12 ns | 0.99 | 59.61 ns |
| interval - interval + take(3) + subscribe + current_thread | 3081.13 ns | 32.63 ns | 34.96 ns | 0.93 | 34.52 ns |
| from array of 1 - create + as_blocking + subscribe + new_thread | 29131.44 ns | 30127.59 ns | 29623.13 ns | 1.02 | 30303.82 ns |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 40940.67 ns | 54164.50 ns | 55497.53 ns | 0.98 | 54030.57 ns |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 3562.80 ns | 131.94 ns | 134.18 ns | 0.98 | 146.00 ns |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 1112.88 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just+filter(true)+subscribe | 859.57 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,2)+skip(1)+subscribe | 997.32 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 863.68 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,2)+first()+subscribe | 1269.45 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,2)+last()+subscribe | 991.90 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just+take_last(1)+subscribe | 1133.02 ns | 18.65 ns | 18.95 ns | 0.98 | 19.89 ns |
| immediate_just(1,2,3)+element_at(1)+subscribe | 829.21 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate scheduler create worker + schedule | 270.68 ns | 1.55 ns | 1.55 ns | 1.00 | 1.55 ns |
| current_thread scheduler create worker + schedule | 366.23 ns | 4.35 ns | 4.35 ns | 1.00 | 4.66 ns |
| current_thread scheduler create worker + schedule + recursive schedule | 817.35 ns | 61.25 ns | 61.58 ns | 0.99 | 61.10 ns |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 844.74 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just+scan(10, std::plus)+subscribe | 895.63 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 2341.81 ns | 125.29 ns | 123.33 ns | 1.02 | 167.81 ns |
| immediate_just+buffer(2)+subscribe | 1531.79 ns | 13.68 ns | 13.68 ns | 1.00 | 17.71 ns |
| immediate_just+window(2)+subscribe + subscsribe inner | 2431.81 ns | 1341.99 ns | 1402.90 ns | 0.96 | 1337.58 ns |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 841.22 ns | - | - | 0.00 | - |
| immediate_just+take_while(true)+subscribe | 829.64 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 2068.94 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 3456.14 ns | 158.49 ns | 158.55 ns | 1.00 | 172.55 ns |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 3735.39 ns | 158.84 ns | 157.61 ns | 1.01 | 161.53 ns |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 135.19 ns | 139.62 ns | 0.97 | 150.42 ns |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 3574.69 ns | 480.54 ns | 374.72 ns | 1.28 | 401.68 ns |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 2158.20 ns | 214.52 ns | 213.70 ns | 1.00 | 212.20 ns |
| immediate_just(immediate_just(1), immediate_just(1)) + concat() + subscribe | 3169.00 ns | 241.28 ns | 226.75 ns | 1.06 | 253.71 ns |
Subjects
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 34.80 ns | 14.79 ns | 14.78 ns | 1.00 | 14.96 ns |
| subscribe 100 observers to publish_subject | 201886.00 ns | 18732.86 ns | 18429.74 ns | 1.02 | 18509.47 ns |
| 100 on_next to 100 observers to publish_subject | 27051.56 ns | 16989.47 ns | 16785.30 ns | 1.01 | 16876.34 ns |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| basic sample | 1474.19 ns | 13.05 ns | 13.06 ns | 1.00 | 22.99 ns |
| basic sample with immediate scheduler | 1459.18 ns | 5.28 ns | 5.28 ns | 1.00 | 16.15 ns |
| mix operators with disposables and without disposables | 6486.21 ns | 1461.87 ns | 1415.22 ns | 1.03 | 1829.85 ns |
| single disposable and looooooong indentity chain | 25011.22 ns | 1005.89 ns | 1071.35 ns | 0.94 | 5200.94 ns |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 998.34 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 2123.96 ns | 988.73 ns | 1022.36 ns | 0.97 | 996.24 ns |
| create(on_error())+retry(1)+subscribe | 601.39 ns | 111.49 ns | 109.46 ns | 1.02 | 112.56 ns |
ci-macos
General
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 385.78 ns | 0.51 ns | 0.96 ns | 0.53 | 0.52 ns |
| Subscribe empty callbacks to empty observable via pipe operator | 394.07 ns | 0.53 ns | 0.96 ns | 0.55 | 0.51 ns |
Sources
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 771.55 ns | 0.38 ns | 0.31 ns | 1.22 | 0.34 ns |
| from array of 1 - create + subscribe + current_thread | 1051.53 ns | 4.10 ns | 4.07 ns | 1.01 | 4.09 ns |
| concat_as_source of just(1 immediate) create + subscribe | 4333.84 ns | 377.06 ns | 175.92 ns | 2.14 | 180.12 ns |
| defer from array of 1 - defer + create + subscribe + immediate | 910.78 ns | 0.34 ns | 0.32 ns | 1.07 | 0.35 ns |
| interval - interval + take(3) + subscribe + immediate | 2353.45 ns | 59.52 ns | 49.64 ns | 1.20 | 56.42 ns |
| interval - interval + take(3) + subscribe + current_thread | 5091.28 ns | 67.60 ns | 29.34 ns | 2.30 | 37.45 ns |
| from array of 1 - create + as_blocking + subscribe + new_thread | 24288.46 ns | 35784.25 ns | 16844.54 ns | 2.12 | 36908.32 ns |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 50855.77 ns | 47084.96 ns | 22330.56 ns | 2.11 | 23212.87 ns |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 5132.32 ns | 196.38 ns | 194.14 ns | 1.01 | 202.44 ns |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 1158.26 ns | 0.34 ns | 0.31 ns | 1.07 | 0.34 ns |
| immediate_just+filter(true)+subscribe | 865.49 ns | 0.34 ns | 0.34 ns | 1.00 | 0.34 ns |
| immediate_just(1,2)+skip(1)+subscribe | 1701.23 ns | 0.34 ns | 0.34 ns | 0.99 | 0.37 ns |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 881.38 ns | 0.35 ns | 0.34 ns | 1.04 | 0.34 ns |
| immediate_just(1,2)+first()+subscribe | 1400.81 ns | 0.34 ns | 0.34 ns | 0.99 | 0.34 ns |
| immediate_just(1,2)+last()+subscribe | 1029.45 ns | 0.99 ns | 0.67 ns | 1.48 | 0.99 ns |
| immediate_just+take_last(1)+subscribe | 1217.26 ns | 0.34 ns | 0.31 ns | 1.07 | 0.34 ns |
| immediate_just(1,2,3)+element_at(1)+subscribe | 864.86 ns | 0.34 ns | 0.31 ns | 1.08 | 0.36 ns |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate scheduler create worker + schedule | 333.16 ns | 1.54 ns | 0.93 ns | 1.66 | 0.51 ns |
| current_thread scheduler create worker + schedule | 956.99 ns | 4.21 ns | 4.44 ns | 0.95 | 4.06 ns |
| current_thread scheduler create worker + schedule + recursive schedule | 769.72 ns | 65.96 ns | 70.80 ns | 0.93 | 65.75 ns |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 1895.07 ns | 2.70 ns | 2.82 ns | 0.96 | 2.70 ns |
| immediate_just+scan(10, std::plus)+subscribe | 992.11 ns | 0.34 ns | 0.34 ns | 0.99 | 0.34 ns |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 2171.39 ns | 209.08 ns | 200.44 ns | 1.04 | 196.48 ns |
| immediate_just+buffer(2)+subscribe | 1069.94 ns | 16.44 ns | 16.49 ns | 1.00 | 16.15 ns |
| immediate_just+window(2)+subscribe + subscsribe inner | 2066.53 ns | 1129.41 ns | 968.56 ns | 1.17 | 1057.28 ns |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 859.70 ns | - | - | 0.00 | - |
| immediate_just+take_while(true)+subscribe | 1902.40 ns | 0.34 ns | 0.31 ns | 1.09 | 0.34 ns |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 1961.08 ns | 2.01 ns | 2.03 ns | 0.99 | 1.91 ns |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 3168.15 ns | 223.22 ns | 224.69 ns | 0.99 | 218.58 ns |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 3632.96 ns | 372.06 ns | 227.43 ns | 1.64 | 219.92 ns |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 222.04 ns | 209.74 ns | 1.06 | 222.55 ns |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 3293.45 ns | 538.34 ns | 500.57 ns | 1.08 | 539.94 ns |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 2136.94 ns | 339.07 ns | 327.96 ns | 1.03 | 333.52 ns |
| immediate_just(immediate_just(1), immediate_just(1)) + concat() + subscribe | 3105.13 ns | 465.32 ns | 371.43 ns | 1.25 | 344.72 ns |
Subjects
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 68.30 ns | 23.67 ns | 23.85 ns | 0.99 | 22.62 ns |
| subscribe 100 observers to publish_subject | 139854.12 ns | 18670.26 ns | 16448.26 ns | 1.14 | 19227.72 ns |
| 100 on_next to 100 observers to publish_subject | 32450.63 ns | 12386.36 ns | 13718.47 ns | 0.90 | 11961.76 ns |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| basic sample | 1340.22 ns | 11.00 ns | 11.48 ns | 0.96 | 29.01 ns |
| basic sample with immediate scheduler | 1360.14 ns | 11.35 ns | 5.56 ns | 2.04 | 10.17 ns |
| mix operators with disposables and without disposables | 6689.23 ns | 1552.64 ns | 1633.79 ns | 0.95 | 1737.31 ns |
| single disposable and looooooong indentity chain | 18386.03 ns | 1735.14 ns | 1754.65 ns | 0.99 | 3770.68 ns |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 1494.80 ns | 0.34 ns | 0.31 ns | 1.08 | 0.34 ns |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 3834.59 ns | 3621.01 ns | 2583.33 ns | 1.40 | 3197.47 ns |
| create(on_error())+retry(1)+subscribe | 734.55 ns | 193.90 ns | 183.82 ns | 1.05 | 186.51 ns |
ci-ubuntu-clang
General
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 270.19 ns | 1.56 ns | 1.56 ns | 1.00 | 0.64 ns |
| Subscribe empty callbacks to empty observable via pipe operator | 269.70 ns | 1.55 ns | 1.55 ns | 1.00 | 0.64 ns |
Sources
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 556.95 ns | 0.31 ns | 0.32 ns | 0.96 | 0.31 ns |
| from array of 1 - create + subscribe + current_thread | 786.94 ns | 4.04 ns | 4.04 ns | 1.00 | 4.04 ns |
| concat_as_source of just(1 immediate) create + subscribe | 2349.33 ns | 130.95 ns | 131.16 ns | 1.00 | 131.11 ns |
| defer from array of 1 - defer + create + subscribe + immediate | 782.39 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| interval - interval + take(3) + subscribe + immediate | 2228.83 ns | 58.69 ns | 58.70 ns | 1.00 | 58.73 ns |
| interval - interval + take(3) + subscribe + current_thread | 3182.07 ns | 31.08 ns | 31.09 ns | 1.00 | 31.70 ns |
| from array of 1 - create + as_blocking + subscribe + new_thread | 29593.13 ns | 30240.61 ns | 32326.26 ns | 0.94 | 30583.30 ns |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 41574.79 ns | 38831.69 ns | 40442.57 ns | 0.96 | 39314.07 ns |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 3678.72 ns | 149.60 ns | 150.03 ns | 1.00 | 149.47 ns |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 1172.10 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just+filter(true)+subscribe | 845.69 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,2)+skip(1)+subscribe | 1096.78 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 875.84 ns | 0.33 ns | 0.33 ns | 1.00 | 0.34 ns |
| immediate_just(1,2)+first()+subscribe | 1389.89 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,2)+last()+subscribe | 1028.70 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just+take_last(1)+subscribe | 1237.81 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just(1,2,3)+element_at(1)+subscribe | 866.42 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate scheduler create worker + schedule | 288.00 ns | 0.64 ns | 0.64 ns | 1.00 | 1.55 ns |
| current_thread scheduler create worker + schedule | 393.26 ns | 4.35 ns | 4.04 ns | 1.08 | 4.04 ns |
| current_thread scheduler create worker + schedule + recursive schedule | 853.22 ns | 55.18 ns | 55.43 ns | 1.00 | 55.51 ns |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 850.74 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
| immediate_just+scan(10, std::plus)+subscribe | 971.93 ns | 0.62 ns | 0.62 ns | 1.00 | 0.33 ns |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 2265.09 ns | 139.53 ns | 136.51 ns | 1.02 | 136.80 ns |
| immediate_just+buffer(2)+subscribe | 1598.52 ns | 14.61 ns | 14.31 ns | 1.02 | 14.60 ns |
| immediate_just+window(2)+subscribe + subscsribe inner | 2544.21 ns | 915.15 ns | 906.69 ns | 1.01 | 895.86 ns |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 847.18 ns | - | - | 0.00 | - |
| immediate_just+take_while(true)+subscribe | 860.07 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 2062.07 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 3326.27 ns | 162.18 ns | 159.45 ns | 1.02 | 156.74 ns |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 3758.21 ns | 140.43 ns | 137.98 ns | 1.02 | 141.31 ns |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 141.77 ns | 143.72 ns | 0.99 | 139.76 ns |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 3441.13 ns | 384.41 ns | 379.72 ns | 1.01 | 381.97 ns |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 2259.47 ns | 203.79 ns | 197.77 ns | 1.03 | 198.65 ns |
| immediate_just(immediate_just(1), immediate_just(1)) + concat() + subscribe | 3248.24 ns | 226.38 ns | 225.23 ns | 1.01 | 224.85 ns |
Subjects
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 54.34 ns | 17.83 ns | 19.84 ns | 0.90 | 20.32 ns |
| subscribe 100 observers to publish_subject | 209163.67 ns | 17416.05 ns | 17529.23 ns | 0.99 | 17370.20 ns |
| 100 on_next to 100 observers to publish_subject | 37659.15 ns | 20317.02 ns | 20226.14 ns | 1.00 | 20259.76 ns |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| basic sample | 1352.27 ns | 11.50 ns | 11.52 ns | 1.00 | 20.81 ns |
| basic sample with immediate scheduler | 1325.88 ns | 5.90 ns | 5.90 ns | 1.00 | 6.52 ns |
| mix operators with disposables and without disposables | 6712.97 ns | 1168.46 ns | 1164.22 ns | 1.00 | 1455.81 ns |
| single disposable and looooooong indentity chain | 27720.02 ns | 1254.43 ns | 1252.38 ns | 1.00 | 4611.10 ns |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 1008.36 ns | 0.31 ns | 0.31 ns | 1.00 | 0.31 ns |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 2177.35 ns | 1164.15 ns | 1156.99 ns | 1.01 | 1184.70 ns |
| create(on_error())+retry(1)+subscribe | 662.26 ns | 138.54 ns | 139.02 ns | 1.00 | 140.73 ns |
ci-windows
General
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 556.07 ns | 2.16 ns | 2.16 ns | 1.00 | 1.85 ns |
| Subscribe empty callbacks to empty observable via pipe operator | 572.10 ns | 2.16 ns | 2.16 ns | 1.00 | 1.85 ns |
Sources
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 1155.57 ns | 4.93 ns | 4.94 ns | 1.00 | 5.55 ns |
| from array of 1 - create + subscribe + current_thread | 1419.82 ns | 15.75 ns | 15.74 ns | 1.00 | 15.45 ns |
| concat_as_source of just(1 immediate) create + subscribe | 3696.46 ns | 174.60 ns | 174.62 ns | 1.00 | 178.05 ns |
| defer from array of 1 - defer + create + subscribe + immediate | 1180.65 ns | 5.24 ns | 5.24 ns | 1.00 | 5.24 ns |
| interval - interval + take(3) + subscribe + immediate | 3736.36 ns | 139.81 ns | 139.74 ns | 1.00 | 142.29 ns |
| interval - interval + take(3) + subscribe + current_thread | 3460.30 ns | 60.15 ns | 59.86 ns | 1.00 | 62.80 ns |
| from array of 1 - create + as_blocking + subscribe + new_thread | 123175.00 ns | 118700.00 ns | 120400.00 ns | 0.99 | 119788.89 ns |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 132655.56 ns | 135675.00 ns | 136975.00 ns | 0.99 | 137275.00 ns |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 5329.67 ns | 206.57 ns | 201.39 ns | 1.03 | 215.13 ns |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 1818.86 ns | 19.42 ns | 19.42 ns | 1.00 | 21.36 ns |
| immediate_just+filter(true)+subscribe | 1610.58 ns | 18.50 ns | 18.51 ns | 1.00 | 21.59 ns |
| immediate_just(1,2)+skip(1)+subscribe | 2008.80 ns | 17.89 ns | 17.91 ns | 1.00 | 21.60 ns |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 1328.95 ns | 20.67 ns | 20.68 ns | 1.00 | 26.86 ns |
| immediate_just(1,2)+first()+subscribe | 2372.16 ns | 18.20 ns | 18.21 ns | 1.00 | 19.43 ns |
| immediate_just(1,2)+last()+subscribe | 1464.98 ns | 19.13 ns | 19.15 ns | 1.00 | 22.84 ns |
| immediate_just+take_last(1)+subscribe | 2023.02 ns | 64.85 ns | 64.93 ns | 1.00 | 70.17 ns |
| immediate_just(1,2,3)+element_at(1)+subscribe | 1622.12 ns | 20.97 ns | 20.98 ns | 1.00 | 21.63 ns |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate scheduler create worker + schedule | 477.39 ns | 4.32 ns | 4.32 ns | 1.00 | 4.32 ns |
| current_thread scheduler create worker + schedule | 648.68 ns | 11.11 ns | 11.11 ns | 1.00 | 11.11 ns |
| current_thread scheduler create worker + schedule + recursive schedule | 1079.50 ns | 103.96 ns | 101.36 ns | 1.03 | 103.12 ns |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 1328.23 ns | 18.80 ns | 18.80 ns | 1.00 | 21.63 ns |
| immediate_just+scan(10, std::plus)+subscribe | 1422.12 ns | 20.96 ns | 20.96 ns | 1.00 | 23.80 ns |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 3837.33 ns | 186.30 ns | 185.11 ns | 1.01 | 223.46 ns |
| immediate_just+buffer(2)+subscribe | 2291.50 ns | 64.26 ns | 63.51 ns | 1.01 | 72.33 ns |
| immediate_just+window(2)+subscribe + subscsribe inner | 3974.07 ns | 1216.92 ns | 1192.15 ns | 1.02 | 1226.57 ns |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 1305.28 ns | 17.57 ns | 17.57 ns | 1.00 | 19.13 ns |
| immediate_just+take_while(true)+subscribe | 1329.92 ns | 18.50 ns | 18.50 ns | 1.00 | 21.60 ns |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 3205.02 ns | 11.10 ns | 11.11 ns | 1.00 | 11.11 ns |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 5054.46 ns | 197.43 ns | 195.27 ns | 1.01 | 222.13 ns |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 5710.11 ns | 186.78 ns | 178.89 ns | 1.04 | 204.81 ns |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 195.15 ns | 194.65 ns | 1.00 | 199.56 ns |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 6119.16 ns | 444.60 ns | 443.65 ns | 1.00 | 489.05 ns |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 3865.37 ns | 538.25 ns | 519.56 ns | 1.04 | 516.04 ns |
| immediate_just(immediate_just(1), immediate_just(1)) + concat() + subscribe | 4853.97 ns | 314.42 ns | 314.38 ns | 1.00 | 325.14 ns |
Subjects
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 36.72 ns | 29.42 ns | 29.27 ns | 1.01 | 30.01 ns |
| subscribe 100 observers to publish_subject | 264250.00 ns | 25013.33 ns | 25450.00 ns | 0.98 | 24830.23 ns |
| 100 on_next to 100 observers to publish_subject | 51881.82 ns | 35939.39 ns | 35922.58 ns | 1.00 | 32997.22 ns |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| basic sample | 1853.25 ns | 96.57 ns | 96.80 ns | 1.00 | 112.11 ns |
| basic sample with immediate scheduler | 1857.89 ns | 66.73 ns | 68.51 ns | 0.97 | 82.35 ns |
| mix operators with disposables and without disposables | 9381.75 ns | 1787.22 ns | 1797.99 ns | 0.99 | 2439.19 ns |
| single disposable and looooooong indentity chain | 25641.46 ns | 1655.18 ns | 1630.37 ns | 1.02 | 6267.21 ns |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 1458.16 ns | 19.11 ns | 19.11 ns | 1.00 | 22.83 ns |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio | rpp no optimization |
|---|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 1896.36 ns | 357.24 ns | 353.06 ns | 1.01 | 380.86 ns |
| create(on_error())+retry(1)+subscribe | 1721.11 ns | 138.15 ns | 137.92 ns | 1.00 | 139.85 ns |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v2 #722 +/- ##
=======================================
Coverage 98.66% 98.66%
=======================================
Files 159 159
Lines 9926 9926
=======================================
Hits 9793 9793
Misses 133 133 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/analyzers.yml (1)
1-9: Add explicit permissions (needed for SARIF upload; safer defaults).
upload-sarif@v3requiressecurity-events: write. Declare permissions at the workflow level.name: Analyzers/sanitizers on: workflow_run: workflows: - "CI v2" types: - completed + +permissions: + contents: read + security-events: write
🧹 Nitpick comments (6)
.github/workflows/analyzers.yml (6)
13-13: Don’t hard‑disable the PVS job; gate it behind a repo variable or branch.
&& falsepermanently skips the analyzer. Prefer a toggle so you can re-enable without code edits.Proposed guard using a repo variable:
- if: github.repository_owner == 'victimsnino' && github.event.workflow_run.conclusion == 'success' && false + if: github.repository_owner == 'victimsnino' && github.event.workflow_run.conclusion == 'success' && (vars.SKIP_ANALYZERS != 'true')Alternative: run only on the v2 branch:
- if: github.repository_owner == 'victimsnino' && github.event.workflow_run.conclusion == 'success' && false + if: github.repository_owner == 'victimsnino' && github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_branch == 'v2'
87-87: Same here: avoid hard‑disabling Sonar/Coverage.Use a variable/branch condition rather than
&& false.- if: github.repository_owner == 'victimsnino' && github.event.workflow_run.conclusion == 'success' && false + if: github.repository_owner == 'victimsnino' && github.event.workflow_run.conclusion == 'success' && (vars.SKIP_ANALYZERS != 'true')
54-57: Remove deprecated apt-key usage.
apt-keyis deprecated on Ubuntu 22.04 and can fail. You already import the keyring via gpg; drop the refresh.- sudo apt-key adv --refresh-keys
40-44: Tighten cache paths to OS-specific directories.This job runs on Ubuntu; caching
/Users/runner/.conan2/(macOS) is wasted bandwidth/storage.path: | ~/.conan2 - /Users/runner/.conan2/Apply the same change in the sonar/coverage job (Lines 114-118).
19-36: Pin third‑party actions to versions/SHAs for supply‑chain safety.
haya14busa/action-workflow_run-status@mainandturtlebrowser/get-conan@mainare floating. Pin to a tag or commit SHA.
15-16: PVS can exceed 10 minutes on cold caches.Consider bumping
timeout-minutesto 20–30 to avoid flaky timeouts when re-enabled.- timeout-minutes: 10 + timeout-minutes: 30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/analyzers.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: benchmarks ci-macos Debug (Optimizations disabled)
- GitHub Check: tests ci-macos Debug (Optimizations disabled)
- GitHub Check: benchmarks ci-macos Release (Optimizations disabled)
- GitHub Check: benchmarks ci-macos Debug
- GitHub Check: tests ci-ubuntu-clang Release
- GitHub Check: tests ci-macos Release
- GitHub Check: benchmarks ci-ubuntu-clang Release
- GitHub Check: benchmarks ci-ubuntu-clang Debug (Optimizations disabled)
- GitHub Check: benchmarks ci-ubuntu-gcc Debug (Optimizations disabled)
- GitHub Check: benchmarks ci-ubuntu-gcc Release
- GitHub Check: benchmarks ci-ubuntu-clang Release (Optimizations disabled)
- GitHub Check: tests ci-ubuntu-clang Debug
- GitHub Check: tests ci-ubuntu-gcc Debug (Optimizations disabled)
- GitHub Check: benchmarks ci-ubuntu-clang Debug
- GitHub Check: tests ci-ubuntu-gcc Release
- GitHub Check: benchmarks ci-ubuntu-gcc Release (Optimizations disabled)
- GitHub Check: Sanitize - asan
- GitHub Check: Sanitize - tsan
- GitHub Check: Sanitize - lsan
- GitHub Check: Sanitize - ubsan
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit