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

bin_to_hex.h: include spdlog.h #2035

Merged
merged 2 commits into from
Aug 9, 2021
Merged

bin_to_hex.h: include spdlog.h #2035

merged 2 commits into from
Aug 9, 2021

Conversation

dmerkushov
Copy link
Contributor

@dmerkushov dmerkushov commented Aug 6, 2021

The goal is to support inclusion of bin_to_hex.h in any order with spdlog.h

PROBLEM FIXED (UNWANTED COMPILATION BEHAVIOR):
Some projects require sorting inclusions in alphabetical order. But in case one includes <spdlog/fmt/bin_to_hex.h> prior to <spdlog/spdlog.h>, multiple compilation errors arise:

  • bin_to_hex.h:35:45: error: ‘size_t’ has not been declared
  • bin_to_hex.h:59:42: error: ‘begin’ is not a member of ‘std’
  • bin_to_hex.h:59:65: error: ‘end’ is not a member of ‘std’
  • bin_to_hex.h:83:5: error: ‘FMT_CONSTEXPR’ does not name a type
  • bin_to_hex.h:115:9: error: ‘SPDLOG_CONSTEXPR’ was not declared in this scope
    and so on...

(My compiler is GCC 10.3)

This PR adds an include of spdlog.h into bin_to_hex.h, so they can be included in any order.

@gabime
Copy link
Owner

gabime commented Aug 7, 2021

Thanks. I think including common.h is sufficiant.

Copy link
Owner

@gabime gabime left a comment

Choose a reason for hiding this comment

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

please add common.h instead spdlog.h

@dmerkushov dmerkushov requested a review from gabime August 9, 2021 16:47
@dmerkushov
Copy link
Contributor Author

please add common.h instead spdlog.h

Done

@gabime gabime merged commit bd99496 into gabime:v1.x Aug 9, 2021
@gabime
Copy link
Owner

gabime commented Aug 9, 2021

Thanks @dmerkushov

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.

2 participants