refactor(ui): inline ImGuiLayer callback type aliases#144
Merged
Conversation
📝 WalkthroughWalkthroughUpdated callback type aliases in ImGuiLayer to use inline Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoInline ImGuiLayer callback type aliases into method signatures
WalkthroughsDescription• Remove callback type aliases from ImGuiLayer header • Inline std::function types directly in method signatures • Update member variable declarations to use inlined types • Remove related type alias assertions from test file Diagramflowchart LR
A["Type Aliases<br/>ParameterChangeCallback<br/>ParameterResetCallback<br/>etc."] -->|"Remove from header"| B["Inlined std::function<br/>in method signatures"]
B -->|"Update declarations"| C["Member variables<br/>use std::function directly"]
D["Test assertions<br/>on type aliases"] -->|"Remove"| E["Cleaner test file"]
File Changes1. src/ui/imgui_layer.hpp
|
Code Review by Qodo
1. Public callback aliases removed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@filter-chain`:
- Line 1: Update the PR description to document the scope of the filter-chain
submodule bump: summarize the functional changes introduced between commits
6d96a36e2cf9a1f39d643fd6c1049fb41aed0391 and
d053e43fdd7f55ebe68edb04975fcd3d1eefa541, list any breaking or API-impacting
changes (public method/signature changes, removed or renamed interfaces,
behavioral changes), and explain why the bump was necessary and how it supports
this PR’s goal so reviewers can assess impact despite the submodule being
private.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e8278ce-47d9-4454-9fc1-8a1aa737fbd5
📒 Files selected for processing (4)
filter-chainsrc/ui/imgui_layer.cppsrc/ui/imgui_layer.hpptests/render/test_filter_boundary_contracts.cpp
💤 Files with no reviewable changes (1)
- tests/render/test_filter_boundary_contracts.cpp
| @@ -1 +1 @@ | |||
| Subproject commit 6d96a36e2cf9a1f39d643fd6c1049fb41aed0391 | |||
| Subproject commit d053e43fdd7f55ebe68edb04975fcd3d1eefa541 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Submodule mapping from .gitmodules =="
if [ -f .gitmodules ]; then
git config -f .gitmodules --get-regexp '^submodule\..*\.(path|url)$' || true
else
echo ".gitmodules not found"
fi
echo
echo "== Gitlink recorded in current HEAD for filter-chain =="
git ls-tree HEAD filter-chain || true
echo
echo "== Expected commit range from review context =="
OLD_COMMIT="6d96a36e2cf9a1f39d643fd6c1049fb41aed0391"
NEW_COMMIT="d053e43fdd7f55ebe68edb04975fcd3d1eefa541"
echo "OLD=${OLD_COMMIT}"
echo "NEW=${NEW_COMMIT}"
echo
if [ -d filter-chain/.git ] || [ -f filter-chain/.git ]; then
echo "== Local submodule log between old and new =="
git -C filter-chain log --oneline --no-merges "${OLD_COMMIT}..${NEW_COMMIT}" || true
echo
echo "== Changed files in submodule range =="
git -C filter-chain diff --name-status "${OLD_COMMIT}..${NEW_COMMIT}" || true
else
echo "Submodule working tree is not initialized in this environment."
echo "Run: git submodule update --init --recursive"
fiRepository: goggles-dev/Goggles
Length of output: 596
🌐 Web query:
goggles-filter-chain repository commits 6d96a36e2cf9a1f39d643fd6c1049fb41aed0391 d053e43fdd7f55ebe68edb04975fcd3d1eefa541
💡 Result:
No public Git repository named "goggles-filter-chain" exists, and the specific commit hashes 6d96a36e2cf9a1f39d643fd6c1049fb41aed0391 and d053e43fdd7f55ebe68edb04975fcd3d1eefa541 do not appear in any indexed sources. Searches across GitHub, general web, company sites, and people profiles yielded no matches for this exact combination. Related projects like chadrem/filter_chain (Ruby gem), K1ngst0m/Goggles (frame streaming), steveoro/goggles_api (swimming API), and Brave Search Goggles exist, but none match the repository name or commits.
Citations:
- 1: https://github.com/chadrem/filter_chain
- 2: https://github.com/K1ngst0m/Goggles
- 3: https://github.com/steveoro/goggles_api
- 4: https://github.com/pipes-and-filters/filters
- 5: https://github.com/steveoro/goggles_main
- 6: https://github.com/brave/goggles-quickstart
- 7: https://github.com/mathieu-lacage/goggles
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to use git ls-remote to get info about the remote repository
echo "== Remote info for filter-chain submodule =="
git ls-remote --heads --tags git@github.com:goggles-dev/goggles-filter-chain.git | head -20 || true
echo
echo "== Attempting to fetch commit messages from remote (if accessible) =="
# Try to fetch the specific commits' info if accessible
OLD_COMMIT="6d96a36e2cf9a1f39d643fd6c1049fb41aed0391"
NEW_COMMIT="d053e43fdd7f55ebe68edb04975fcd3d1eefa541"
# Try git fetch-pack to get information about specific commits
echo "OLD_COMMIT=${OLD_COMMIT}"
echo "NEW_COMMIT=${NEW_COMMIT}"
# Alternative: check if any remote is configured that might have filter-chain history
echo
echo "== Configured remotes =="
git remote -v || trueRepository: goggles-dev/Goggles
Length of output: 609
Document the scope of the filter-chain submodule bump in the PR description.
The filter-chain submodule repository is not publicly accessible, making it impossible to review what changed between the old and new commits directly from GitHub. Because of this access restriction, it is essential to include in this PR's description:
- A summary of what changed in the
filter-chainsubmodule (commit6d96a36e2cf9a1f39d643fd6c1049fb41aed0391→d053e43fdd7f55ebe68edb04975fcd3d1eefa541) - Any breaking or API-impact changes
- Why this bump was necessary and how it relates to the PR objective
This ensures traceability and allows reviewers to understand the impact of vendored dependency updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@filter-chain` at line 1, Update the PR description to document the scope of
the filter-chain submodule bump: summarize the functional changes introduced
between commits 6d96a36e2cf9a1f39d643fd6c1049fb41aed0391 and
d053e43fdd7f55ebe68edb04975fcd3d1eefa541, list any breaking or API-impacting
changes (public method/signature changes, removed or renamed interfaces,
behavioral changes), and explain why the bump was necessary and how it supports
this PR’s goal so reviewers can assess impact despite the submodule being
private.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| PreChainState prechain; | ||
| }; | ||
|
|
||
| using ParameterChangeCallback = |
There was a problem hiding this comment.
1. Public callback aliases removed 🐞 Bug ⚙ Maintainability
Deleting the public callback type aliases in goggles::ui will break any downstream code that
referenced goggles::ui::{ParameterChangeCallback, ...} even though the underlying std::function
signatures are unchanged. This is a source/ABI-facing API break because the header is exposed via
the goggles_ui target’s PUBLIC include directories.
Agent Prompt
### Issue description
The PR removes the public callback type aliases (e.g., `goggles::ui::ParameterChangeCallback`) from `src/ui/imgui_layer.hpp`. Any downstream code that referenced these names will now fail to compile.
### Issue Context
Even if you prefer inline `std::function` types in method signatures/members, you can keep the aliases purely as compatibility shims.
### Fix Focus Areas
- src/ui/imgui_layer.hpp[70-120]
- Reintroduce the removed `using ...Callback = std::function<...>;` aliases in `namespace goggles::ui` (optionally mark them `[[deprecated]]` with a clear message).
- Keep the new inline `std::function` signatures if that’s the refactor goal; the aliases can remain unused internally but preserve downstream compatibility.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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