chore: remove dead code, redundant comments, and test bloat#139
Merged
chore: remove dead code, redundant comments, and test bloat#139
Conversation
- Remove serializer.hpp and test_serializer.cpp (zero production call sites) - Inline trivial ResultPtr wrappers at 13 call sites across four modules - Audit and strip self-documenting Doxygen comments from public headers - Cut queue stress tests, memory ordering benchmarks, and zero-copy patterns - Tighten docstring policy §5.4 with concrete MUST/MUST NOT rules
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughTightens public docstring policy, removes many Doxygen comments, changes several factory functions to use braced Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 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 |
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Review Summary by QodoRemove dead code, redundant comments, and test bloat; inline ResultPtr wrappers
WalkthroughsDescription• Replace make_result_ptr() wrapper calls with direct brace initialization across 6 modules • Strip redundant Doxygen comments from public headers per updated policy §5.4 • Remove unused serializer.hpp and test_serializer.cpp (zero production call sites) • Cut queue stress tests, memory ordering benchmarks, and zero-copy patterns • Tighten docstring policy with explicit MUST/MUST NOT rules for self-documenting code Diagramflowchart LR
A["ResultPtr Wrapper Calls"] -->|"Inline to braces"| B["Direct Initialization"]
C["Redundant Doxygen Comments"] -->|"Strip per policy"| D["Self-Documenting Code"]
E["Dead Code Files"] -->|"Remove"| F["Cleaner Codebase"]
G["Stress/Benchmark Tests"] -->|"Cut"| H["Focused Test Suite"]
I["Policy §5.4"] -->|"Tighten MUST/MUST NOT"| J["Explicit Guidance"]
File Changes1. src/app/application.cpp
|
Code Review by Qodo
1. Changelog section numbering broken
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/util/test_error.cpp (1)
7-18:⚠️ Potential issue | 🟡 MinorEnum regression coverage is now incomplete.
This now checks only a subset of distinctness;
file_not_foundcan still equalparse_errorwithout failing. It also drops explicitokinvariant coverage (value/name), which weakens safety for common success-code assumptions.Suggested test patch
TEST_CASE("ErrorCode enum values are correct", "[error]") { + REQUIRE(static_cast<int>(ErrorCode::ok) == 0); REQUIRE(ErrorCode::file_not_found != ErrorCode::vulkan_init_failed); REQUIRE(ErrorCode::vulkan_init_failed != ErrorCode::parse_error); + REQUIRE(ErrorCode::file_not_found != ErrorCode::parse_error); } TEST_CASE("error_code_name returns correct strings", "[error]") { + REQUIRE(std::string(error_code_name(ErrorCode::ok)) == "ok"); REQUIRE(std::string(error_code_name(ErrorCode::file_not_found)) == "file_not_found"); REQUIRE(std::string(error_code_name(ErrorCode::vulkan_init_failed)) == "vulkan_init_failed");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/util/test_error.cpp` around lines 7 - 18, The tests in tests/util/test_error.cpp only assert a few distinct ErrorCode values and drop checks for the common success code; update the TEST_CASEs to comprehensively assert distinctness among all enum members and reintroduce explicit checks for the success/ok invariant: verify ErrorCode::ok has the expected unique value and that error_code_name(ErrorCode::ok) returns "ok"; also expand the distinctness assertions to ensure file_not_found != parse_error and all other enum pairs that must be unique, using the ErrorCode enum and error_code_name helper to locate the symbols to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/project_policies.md`:
- Around line 317-329: The changelog headings are misnumbered (you have "###
12.1 v1.2 -> v1.3" followed by "### 12.3 v1.1 -> v1.2"), so update the section
headers to maintain sequential numbering: rename "### 12.3 v1.1 -> v1.2" to "###
12.2 v1.1 -> v1.2" and adjust "### 12.4 v1.0 -> v1.1" to "### 12.3 v1.0 -> v1.1"
(or insert a new "### 12.2" entry if a distinct entry was omitted) so the
headings (12.1, 12.2, 12.3, ...) are consecutive and reflect the intended
versions.
In `@filter-chain`:
- Line 1: A submodule version bump may introduce API/behavior changes affecting
filter_chain_controller, the Vulkan backend, and the UI layer; manually compare
the upstream commit diff between the old and new submodule revisions to identify
any changed function signatures, removed/renamed types, altered semantics, or
performance-related changes, then update call sites in filter_chain_controller,
Vulkan backend integration points, and UI adapters accordingly, adjust or add
unit/integration tests (including test_filter_chain_retarget.cpp and
test_filter_boundary_contracts.cpp) to cover any new behaviors, and run the full
test suite and representative integration scenarios to confirm no regressions
before merging.
---
Outside diff comments:
In `@tests/util/test_error.cpp`:
- Around line 7-18: The tests in tests/util/test_error.cpp only assert a few
distinct ErrorCode values and drop checks for the common success code; update
the TEST_CASEs to comprehensively assert distinctness among all enum members and
reintroduce explicit checks for the success/ok invariant: verify ErrorCode::ok
has the expected unique value and that error_code_name(ErrorCode::ok) returns
"ok"; also expand the distinctness assertions to ensure file_not_found !=
parse_error and all other enum pairs that must be unique, using the ErrorCode
enum and error_code_name helper to locate the symbols to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b931961-6450-4ca3-b1a7-948a89ddd9cf
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
docs/project_policies.mdfilter-chainsrc/app/application.cppsrc/app/application.hppsrc/compositor/compositor_server.cppsrc/compositor/compositor_server.hppsrc/render/backend/filter_chain_controller.hppsrc/render/backend/vulkan_backend.cppsrc/render/backend/vulkan_backend.hppsrc/render/backend/vulkan_context.hppsrc/render/backend/vulkan_debug.hppsrc/ui/imgui_layer.cppsrc/ui/imgui_layer.hppsrc/util/config.hppsrc/util/job_system.hppsrc/util/logging.hppsrc/util/paths.hppsrc/util/queues.hppsrc/util/scale_mode.hppsrc/util/serializer.hppsrc/util/unique_fd.hpptests/CMakeLists.txttests/util/test_error.cpptests/util/test_queues.cpptests/util/test_serializer.cpp
💤 Files with no reviewable changes (13)
- src/render/backend/vulkan_context.hpp
- src/util/unique_fd.hpp
- src/util/scale_mode.hpp
- src/render/backend/vulkan_debug.hpp
- src/app/application.hpp
- src/util/paths.hpp
- tests/CMakeLists.txt
- src/render/backend/filter_chain_controller.hpp
- src/util/queues.hpp
- src/ui/imgui_layer.hpp
- tests/util/test_serializer.cpp
- tests/util/test_queues.cpp
- src/util/serializer.hpp
Comment on lines
+317
to
+329
| ### 12.1 v1.2 -> v1.3 | ||
|
|
||
| - Replaced permissive SHOULD guidance in section 5.4 with explicit MUST/MUST NOT rules for Doxygen docstrings. | ||
| - Stripped redundant `///` comments from public headers to align with the updated policy. | ||
|
|
||
| ### 12.3 v1.1 -> v1.2 | ||
|
|
||
| - Removed policy text that is now directly gated by Semgrep (`using namespace` in headers, raw `new`/`delete`, banned Vulkan RAII wrappers, render-path thread bans, std-stream/`printf` logging bans). | ||
| - Moved identifier naming specifics to the repository `clang-tidy` configuration and kept only the high-level policy contract plus scoped-override allowance. | ||
| - Expanded Semgrep logging coverage to include `printf` so the removed overlap remains CI-gated. | ||
| - Updated enforcement tags and matrix to call out Semgrep-owned policy checks explicitly. | ||
|
|
||
| ### 12.2 v1.0 -> v1.1 | ||
| ### 12.4 v1.0 -> v1.1 |
There was a problem hiding this comment.
Changelog section numbering is inconsistent.
The changelog sections jump from 12.1 to 12.3, skipping 12.2. This appears to be a copy-paste error from reindexing after adding the new v1.2 → v1.3 entry.
📝 Proposed fix
### 12.1 v1.2 -> v1.3
- Replaced permissive SHOULD guidance in section 5.4 with explicit MUST/MUST NOT rules for Doxygen docstrings.
- Stripped redundant `///` comments from public headers to align with the updated policy.
-### 12.3 v1.1 -> v1.2
+### 12.2 v1.1 -> v1.2
- Removed policy text that is now directly gated by Semgrep (`using namespace` in headers, raw `new`/`delete`, banned Vulkan RAII wrappers, render-path thread bans, std-stream/`printf` logging bans).
- Moved identifier naming specifics to the repository `clang-tidy` configuration and kept only the high-level policy contract plus scoped-override allowance.
- Expanded Semgrep logging coverage to include `printf` so the removed overlap remains CI-gated.
- Updated enforcement tags and matrix to call out Semgrep-owned policy checks explicitly.
-### 12.4 v1.0 -> v1.1
+### 12.3 v1.0 -> v1.1
- Rewrote document to RFC-style normative format.📝 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
| ### 12.1 v1.2 -> v1.3 | |
| - Replaced permissive SHOULD guidance in section 5.4 with explicit MUST/MUST NOT rules for Doxygen docstrings. | |
| - Stripped redundant `///` comments from public headers to align with the updated policy. | |
| ### 12.3 v1.1 -> v1.2 | |
| - Removed policy text that is now directly gated by Semgrep (`using namespace` in headers, raw `new`/`delete`, banned Vulkan RAII wrappers, render-path thread bans, std-stream/`printf` logging bans). | |
| - Moved identifier naming specifics to the repository `clang-tidy` configuration and kept only the high-level policy contract plus scoped-override allowance. | |
| - Expanded Semgrep logging coverage to include `printf` so the removed overlap remains CI-gated. | |
| - Updated enforcement tags and matrix to call out Semgrep-owned policy checks explicitly. | |
| ### 12.2 v1.0 -> v1.1 | |
| ### 12.4 v1.0 -> v1.1 | |
| ### 12.1 v1.2 -> v1.3 | |
| - Replaced permissive SHOULD guidance in section 5.4 with explicit MUST/MUST NOT rules for Doxygen docstrings. | |
| - Stripped redundant `///` comments from public headers to align with the updated policy. | |
| ### 12.2 v1.1 -> v1.2 | |
| - Removed policy text that is now directly gated by Semgrep (`using namespace` in headers, raw `new`/`delete`, banned Vulkan RAII wrappers, render-path thread bans, std-stream/`printf` logging bans). | |
| - Moved identifier naming specifics to the repository `clang-tidy` configuration and kept only the high-level policy contract plus scoped-override allowance. | |
| - Expanded Semgrep logging coverage to include `printf` so the removed overlap remains CI-gated. | |
| - Updated enforcement tags and matrix to call out Semgrep-owned policy checks explicitly. | |
| ### 12.3 v1.0 -> v1.1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/project_policies.md` around lines 317 - 329, The changelog headings are
misnumbered (you have "### 12.1 v1.2 -> v1.3" followed by "### 12.3 v1.1 ->
v1.2"), so update the section headers to maintain sequential numbering: rename
"### 12.3 v1.1 -> v1.2" to "### 12.2 v1.1 -> v1.2" and adjust "### 12.4 v1.0 ->
v1.1" to "### 12.3 v1.0 -> v1.1" (or insert a new "### 12.2" entry if a distinct
entry was omitted) so the headings (12.1, 12.2, 12.3, ...) are consecutive and
reflect the intended versions.
| @@ -1 +1 @@ | |||
| Subproject commit 0873141f783efaf5ae8930c52521d14b5786fe76 | |||
| Subproject commit 6d96a36e2cf9a1f39d643fd6c1049fb41aed0391 | |||
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate submodule URL =="
if [ -f .gitmodules ]; then
cat .gitmodules
else
echo ".gitmodules not found; cannot verify submodule remote."
exit 1
fi
echo
echo "== Extract filter-chain URL =="
sub_url="$(git config -f .gitmodules --get submodule.filter-chain.url || true)"
if [ -z "${sub_url}" ]; then
echo "Submodule 'filter-chain' URL not found in .gitmodules"
exit 1
fi
echo "filter-chain URL: ${sub_url}"
echo
echo "== Ensure old/new commits are known in submodule repo =="
old_commit="0873141f"
new_commit="6d96a36e2cf9a1f39d643fd6c1049fb41aed0391"
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
git clone --filter=blob:none --no-checkout "$sub_url" "$tmpdir/repo" >/dev/null 2>&1
git -C "$tmpdir/repo" rev-parse --verify "${new_commit}^{commit}" >/dev/null
echo "New commit exists: ${new_commit}"
# Old hash is abbreviated in summary; resolve if possible
if git -C "$tmpdir/repo" rev-parse --verify "${old_commit}^{commit}" >/dev/null 2>&1; then
full_old="$(git -C "$tmpdir/repo" rev-parse "${old_commit}^{commit}")"
echo "Old commit resolved: ${full_old}"
echo
echo "== Upstream commit list between old and new =="
git -C "$tmpdir/repo" log --oneline "${full_old}..${new_commit}" | sed -n '1,80p'
else
echo "Could not resolve old abbreviated commit '${old_commit}' in upstream repo."
echo "Please provide full old hash from base branch gitlink for exact compare."
fi
echo
echo "== Find in-repo references to filter-chain integration points =="
rg -n -C2 'filter-chain|ResultPtr|serializer|zero-copy|memory ordering|queue stress' || trueRepository: goggles-dev/Goggles
Repository: goggles-dev/Goggles
Exit code: 128
stdout:
== Locate submodule URL ==
[submodule "filter-chain"]
path = filter-chain
url = git@github.com:goggles-dev/goggles-filter-chain.git
== Extract filter-chain URL ==
filter-chain URL: git@github.com:goggles-dev/goggles-filter-chain.git
== Ensure old/new commits are known in submodule repo ==
Script executed:
rg -l 'filter-chain|FilterChain' --type-list | head -20Repository: goggles-dev/Goggles
Repository: goggles-dev/Goggles
Exit code: 0
stderr:
error: The argument '<PATTERN>' cannot be used with '--type-list'
USAGE:
rg [OPTIONS] PATTERN [PATH ...]
rg [OPTIONS] -e PATTERN ... [PATH ...]
rg [OPTIONS] -f PATTERNFILE ... [PATH ...]
rg [OPTIONS] --files [PATH ...]
rg [OPTIONS] --type-list
command | rg [OPTIONS] PATTERN
rg [OPTIONS] --help
rg [OPTIONS] --version
For more information try --help
Script executed:
fd -type f -name '*.rs' -o -name '*.toml' -o -name '*.md' | xargs rg -l 'filter.?chain' 2>/dev/null | head -20Repository: goggles-dev/Goggles
Repository: goggles-dev/Goggles
Exit code: 0
stdout:
tests/visual/test_shader_basic.cpp
scripts/task/validate-installed-consumers.sh
scripts/task/ci.sh
scripts/generate_shader_report.sh
tests/render/test_vulkan_backend_subsystem_contracts.cpp
tests/render/test_filter_chain_retarget.cpp
tests/render/test_filter_boundary_contracts.cpp
tests/golden/README.md
tests/CMakeLists.txt
src/ui/imgui_layer.hpp
src/render/CMakeLists.txt
src/compositor/compositor_server.hpp
src/ui/CMakeLists.txt
src/ui/imgui_layer.cpp
src/render/backend/CMakeLists.txt
src/render/backend/filter_chain_controller.hpp
src/render/backend/filter_chain_controller.cpp
src/render/backend/vulkan_backend.cpp
src/render/backend/vulkan_context.hpp
src/render/backend/vulkan_backend.hpp
stderr:
error: 'ype' isn't a valid value for '--type <filetype>'
[possible values: file, directory, symlink, executable, empty, socket, pipe]
For more information try '--help'
Script executed:
head -100 src/render/backend/filter_chain_controller.hppRepository: goggles-dev/Goggles
Repository: goggles-dev/Goggles
Exit code: 0
stdout:
`#pragma` once
`#include` <array>
`#include` <atomic>
`#include` <filesystem>
`#include` <functional>
`#include` <future>
`#include` <goggles/filter_chain.h>
`#include` <goggles/filter_chain.hpp>
`#include` <goggles/filter_chain/filter_controls.hpp>
`#include` <vector>
`#include` <vulkan/vulkan.h>
`#include` <vulkan/vulkan.hpp>
namespace goggles::render::backend_internal {
struct FilterChainController {
struct ControlOverride {
goggles::fc::FilterControlId control_id = 0;
float value = 0.0F;
};
struct VulkanDeviceInfo {
VkPhysicalDevice physical_device = VK_NULL_HANDLE;
VkDevice device = VK_NULL_HANDLE;
VkQueue graphics_queue = VK_NULL_HANDLE;
uint32_t graphics_queue_family_index = 0;
std::string cache_dir;
};
struct ChainConfig {
VkFormat target_format = VK_FORMAT_UNDEFINED;
uint32_t frames_in_flight = 1;
uint32_t initial_stage_mask = GOGGLES_FC_STAGE_MASK_ALL;
uint32_t initial_prechain_width = 0;
uint32_t initial_prechain_height = 0;
};
struct RecordParams {
VkCommandBuffer command_buffer = VK_NULL_HANDLE;
VkImage source_image = VK_NULL_HANDLE;
VkImageView source_view = VK_NULL_HANDLE;
uint32_t source_width = 0;
uint32_t source_height = 0;
VkImageView target_view = VK_NULL_HANDLE;
uint32_t target_width = 0;
uint32_t target_height = 0;
uint32_t frame_index = 0;
uint32_t scale_mode = GOGGLES_FC_SCALE_MODE_STRETCH;
uint32_t integer_scale = 1;
};
struct AdapterBuildConfig {
VulkanDeviceInfo device_info;
ChainConfig chain_config;
};
struct OutputTarget {
vk::Format format = vk::Format::eUndefined;
vk::Extent2D extent;
};
struct PrechainResolutionConfig {
vk::Extent2D requested_resolution;
};
[[nodiscard]] auto recreate_filter_chain(const AdapterBuildConfig& config) -> Result<void>;
[[nodiscard]] auto retarget_filter_chain(const OutputTarget& output_target) -> Result<void>;
void shutdown(const std::function<void()>& wait_for_gpu_idle);
void load_shader_preset(
const std::filesystem::path& new_preset_path,
const std::function<void()>& wait_for_safe_rebuild = std::function<void()>{});
[[nodiscard]] auto reload_shader_preset(std::filesystem::path new_preset_path,
AdapterBuildConfig config) -> Result<void>;
void advance_frame();
void check_pending_chain_swap(const std::function<void()>& wait_all_frames);
void cleanup_retired_adapters();
void
set_stage_policy(bool prechain_enabled, bool effect_stage_enabled,
const std::function<void()>& wait_for_safe_rebuild = std::function<void()>{});
void set_prechain_resolution(
const PrechainResolutionConfig& config,
const std::function<void()>& wait_for_safe_rebuild = std::function<void()>{});
[[nodiscard]] auto handle_resize(vk::Extent2D target_extent) -> Result<void>;
[[nodiscard]] auto record(const RecordParams& record_params) -> Result<void>;
[[nodiscard]] auto current_prechain_resolution() const -> vk::Extent2D;
[[nodiscard]] auto current_preset_path() const -> const std::filesystem::path& {
return preset_path;
}
[[nodiscard]] auto has_filter_chain() const -> bool {
return static_cast<bool>(active_slot.chain);
}
[[nodiscard]] auto consume_chain_swapped() -> bool {
return chain_swapped.exchange(false, std::memory_order_acq_rel);
}
Script executed:
head -50 src/render/backend/filter_chain_controller.cppRepository: goggles-dev/Goggles
Repository: goggles-dev/Goggles
Exit code: 0
stdout:
`#include` "filter_chain_controller.hpp"
`#include` <algorithm>
`#include` <chrono>
`#include` <cstdint>
`#include` <cstring>
`#include` <filesystem>
`#include` <format>
`#include` <goggles/profiling.hpp>
`#include` <limits>
`#include` <string>
`#include` <string_view>
`#include` <util/job_system.hpp>
`#include` <util/logging.hpp>
`#include` <vector>
namespace goggles::render::backend_internal {
namespace {
// ---------------------------------------------------------------------------
// Slot-level helper types
// ---------------------------------------------------------------------------
struct ControlSnapshot {
goggles::fc::FilterControlId control_id = 0;
float value = 0.0F;
};
// ---------------------------------------------------------------------------
// Path / encoding helpers
// ---------------------------------------------------------------------------
auto to_utf8_bytes(const std::filesystem::path& path) -> std::string {
const auto utf8 = path.u8string();
return {reinterpret_cast<const char*>(utf8.c_str()), utf8.size()};
}
// ---------------------------------------------------------------------------
// Stage / control translation helpers
// ---------------------------------------------------------------------------
auto to_filter_stage(uint32_t fc_stage) -> goggles::fc::FilterControlStage {
switch (fc_stage) {
case GOGGLES_FC_STAGE_PRECHAIN:
return goggles::fc::FilterControlStage::prechain;
case GOGGLES_FC_STAGE_EFFECT:
default:
return goggles::fc::FilterControlStage::effect;
}
Script executed:
head -50 tests/render/test_filter_chain_retarget.cppRepository: goggles-dev/Goggles
Repository: goggles-dev/Goggles
Exit code: 0
stdout:
`#include` "render/backend/filter_chain_controller.hpp"
`#include` <atomic>
`#include` <catch2/catch_approx.hpp>
`#include` <catch2/catch_test_macros.hpp>
`#include` <chrono>
`#include` <filesystem>
`#include` <fstream>
`#include` <limits>
`#include` <optional>
`#include` <spdlog/spdlog.h>
`#include` <string>
`#include` <thread>
`#include` <util/logging.hpp>
`#include` <vector>
`#include` <vulkan/vulkan.hpp>
namespace {
constexpr uint32_t TEST_SYNC_INDICES = 2u;
auto make_cache_dir() -> std::filesystem::path {
static std::atomic<uint64_t> counter{0u};
auto path = std::filesystem::temp_directory_path() /
("goggles_filter_chain_retarget_" + std::to_string(counter.fetch_add(1u)));
std::filesystem::create_directories(path);
return path;
}
auto read_text_file(const std::filesystem::path& path) -> std::optional<std::string> {
std::ifstream file(path);
if (!file.is_open()) {
return std::nullopt;
}
return std::string(std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>());
}
struct VulkanRuntimeFixture {
VulkanRuntimeFixture() {
VkApplicationInfo app_info{};
app_info.sType = VK_STRUCTURE_TYPE_APPLICATION_INFO;
app_info.pApplicationName = "goggles_filter_chain_retarget_tests";
app_info.applicationVersion = VK_MAKE_VERSION(1, 0, 0);
app_info.apiVersion = VK_API_VERSION_1_3;
VkInstanceCreateInfo instance_info{};
instance_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO;
instance_info.pApplicationInfo = &app_info;
if (vkCreateInstance(&instance_info, nullptr, &m_instance) != VK_SUCCESS) {
return;
Script executed:
cat src/render/CMakeLists.txt | grep -A 10 -B 10 filterRepository: goggles-dev/Goggles
Repository: goggles-dev/Goggles
Exit code: 0
stdout:
target_include_directories(goggles_render PUBLIC
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_SOURCE_DIR}/src
)
# Link deps here so downstream users only need to link goggles_render.
target_link_libraries(goggles_render PUBLIC
goggles_util
Vulkan::Vulkan
SDL3::SDL3
GogglesFilterChain::goggles-filter-chain
)
target_compile_definitions(goggles_render PUBLIC
VULKAN_HPP_NO_EXCEPTIONS
VULKAN_HPP_DISPATCH_LOADER_DYNAMIC=1
)
goggles_enable_clang_tidy(goggles_render)
goggles_enable_sanitizers(goggles_render)
goggles_enable_profiling(goggles_render)
Verify submodule bump impact before merge.
This single-line pointer change introduces a dependency update with multiple integration points in the codebase (filter_chain_controller, Vulkan backend, UI layer). Please manually review the upstream commit diff between the old and new versions to identify any API, behavior, or performance changes, and confirm that existing tests cover the impacted surfaces.
While the test suite (test_filter_chain_retarget.cpp, test_filter_boundary_contracts.cpp) provides some assurance against obvious breakage, manual upstream inspection is needed to catch semantic or subtle compatibility issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@filter-chain` at line 1, A submodule version bump may introduce API/behavior
changes affecting filter_chain_controller, the Vulkan backend, and the UI layer;
manually compare the upstream commit diff between the old and new submodule
revisions to identify any changed function signatures, removed/renamed types,
altered semantics, or performance-related changes, then update call sites in
filter_chain_controller, Vulkan backend integration points, and UI adapters
accordingly, adjust or add unit/integration tests (including
test_filter_chain_retarget.cpp and test_filter_boundary_contracts.cpp) to cover
any new behaviors, and run the full test suite and representative integration
scenarios to confirm no regressions before merging.
K1ngst0m
force-pushed
the
dev/chore-removed-stale
branch
from
2188c12 to
a14a1bd
Compare
K1ngst0m
force-pushed
the
dev/chore-removed-stale
branch
from
a14a1bd to
865b64f
Compare
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
Breaking Changes
load_config()anddefault_config().Bug Fixes & Improvements
Documentation
Chores