Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Introduce CATCH_CONFIG_PREFIX_MESSAGES to only prefix a few logging related macros. #2544

Merged
merged 14 commits into from
Jul 19, 2023

Conversation

Rijom
Copy link
Contributor

@Rijom Rijom commented Oct 7, 2022

Description

The macro CATCH_CONFIG_PREFIX_ALL allows to prefix all of catch2's macros with CATCH_. This commit adds some granularity by introducing a new macro: CATCH_CONFIG_PREFIX_MESSAGES. It only prefixes a few debug message macros.

Why? It's rather usual that logging frameworks already use INFO/WARN macros themselves. This allows a user to avoid clashes without having to use the prefix everywhere else.

GitHub Issues

std::min is defined in algorithm provides std::min. It appears to be transitively included for most platforms. For VxWorks however this explicit include is required.
Add missing include for VxWorks build.
… macros only.

In contrast to CATCH_CONFIG_PREFIX_ALL, this will only prefix the following macros:
I.e. INFO, UNSCOPED_INFO, WARN and CATCH_CAPTURE

This is mainly useful for codebases that use INFO or WARN for their own logging macros.
@Rijom Rijom marked this pull request as ready for review October 7, 2022 17:29
@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #2544 (14b2fee) into devel (6e79e68) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            devel    #2544      +/-   ##
==========================================
- Coverage   91.20%   91.19%   -0.01%     
==========================================
  Files         192      192              
  Lines        7843     7843              
==========================================
- Hits         7153     7152       -1     
- Misses        690      691       +1     

@Vertexwahn
Copy link
Contributor

Would be nice if you can add it also here: expand_template( expand_template( to ensure the Bazel build does not break.

@Rijom
Copy link
Contributor Author

Rijom commented Dec 23, 2022

Would be nice if you can add it also here: expand_template( expand_template( to ensure the Bazel build does not break.

Like so?

@Rijom Rijom changed the title Introduce CATCH_CONFIG_PREFIX_MESSAGES Introduce CATCH_CONFIG_PREFIX_MESSAGES to only prefix a few logging related macros. Dec 23, 2022
@Rijom
Copy link
Contributor Author

Rijom commented Mar 6, 2023

@horenmar Any comments regarding this PR?

Copy link
Member

@horenmar horenmar left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to get around to this PR, but it happens to be in the sweet spot where there is no actual reason to reject it beyond me not seeing the value (the maintenance overhead and extra complexity is small enough), but also me not having a motivation to merge it (because I do not see the value & even if tiny, there are still costs).

In the end I decided to merge it, if one small change happens.

src/catch2/catch_message.hpp Outdated Show resolved Hide resolved
@Rijom Rijom requested a review from horenmar July 19, 2023 08:56
@horenmar horenmar merged commit 4acc518 into catchorg:devel Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants