Skip to content

Fix DEBUG_CRC compile errors and add RTS_DEBUG_CRC cmake option #771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix DEBUG_CRC compile errors and add RTS_DEBUG_CRC cmake option #771

wants to merge 3 commits into from
+25 −30

Conversation

xezon
Copy link

@xezon xezon commented Apr 26, 2025

Merge with Rebase

This change fixes compile errors from enabling DEBUG_CRC and adds a RTS_DEBUG_CRC cmake option.

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Build Anything related to building, compiling Debug Is mostly debug functionality labels Apr 26, 2025
@xezon xezon added this to the Code foundation build up milestone Apr 26, 2025
@xezon xezon requested a review from helmutbuhler April 26, 2025 20:55
@xezon xezon force-pushed the xezon/add-debug-crc-cmake-option branch from f38860a to 8e8fd01 Compare April 27, 2025 08:42
@xezon
Copy link
Author

xezon commented Apr 27, 2025

Fixed the Debug compile error.

helmutbuhler
helmutbuhler approved these changes Apr 27, 2025
View reviewed changes
Copy link

@helmutbuhler helmutbuhler left a comment

Choose a reason for hiding this comment

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

I'd still prefer not to merge this, because there is no benefit to enable crc logging and disable logging. And allowing these defines to be separate will make future additional crc logs more annoying to add. But if you think this is useful go ahead.

cmake/config-debug.cmake
@@ -10,34 +10,33 @@ set_property(CACHE RTS_DEBUG_STACKTRACE PROPERTY STRINGS DEFAULT ON OFF)
set(RTS_DEBUG_PROFILE "DEFAULT" CACHE STRING "Enables debug profiling. When DEFAULT, this option is enabled with DEBUG or INTERNAL")
set_property(CACHE RTS_DEBUG_PROFILE PROPERTY STRINGS DEFAULT ON OFF)

option(RTS_DEBUG_INCLUDE_DEBUG_LOG_IN_CRC_LOG "Include normal debug log in crc log" OFF)
set(RTS_DEBUG_CRC "DEFAULT" CACHE STRING "Enables debug CRC testing. When DEFAULT, this option is enabled with DEBUG_LOGGING")
Copy link

Choose a reason for hiding this comment

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

debug logging, not testing

@xezon
Copy link
Author

xezon commented Apr 27, 2025

Ok then how about we just fix the theoretical compile errors, but not add the cmake option yet?

@helmutbuhler
Copy link

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@helmutbuhler helmutbuhler helmutbuhler approved these changes

Assignees
No one assigned
Labels
Build Anything related to building, compiling Debug Is mostly debug functionality Minor Severity: Minor < Major < Critical < Blocker
Projects
None yet
Milestone
Code foundation build up
Development

Successfully merging this pull request may close these issues.
2 participants
@xezon @helmutbuhler