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

#213 Fix bug with {fmt} integration #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

YarikTH
Copy link
Contributor

@YarikTH YarikTH commented Sep 26, 2023

This should be small

Small pull requests are great and easy for me to understand and accept
Please try prefix every commits in the pull request with Arlo's git notation

Prefix Meaning
e development enviroment only - not production
d documentation only
t tests only
R!! Refactoring
B!! Bug Fix
F!! New Feature

But it's not small!

Then you should setup a remote pairing session with Llewellyn ( [email protected] )
Usually the sessions are between 45-90 minutes.

assuming you still feel it is small, please include

Description

See linked issue #213.

TL;DR {fmt} integration is triggered by FMT_VERSION define that is provided by <fmt/core.h>, while feature itself is using fmt::to_string() that is defined in <fmt/format.h> that causes a bug.

The solution

Include <fmt/format.h> in case {fmt} integration is triggered, so even if user included only <fmt/core.h>, the library provides itself with needed header to work correctly.

Notation

I prefer lots of very small commits prefixed with Arlo's git notation

@YarikTH
Copy link
Contributor Author

YarikTH commented Sep 26, 2023

Sorry, I made a commit before reading about Arlo git notation. I can modify its message if needed

@claremacrae
Copy link
Collaborator

Thank you very much indeed for this.

We are currently trying to get the builds passing again, after which we will test, review and merge this.

@YarikTH
Copy link
Contributor Author

YarikTH commented Mar 28, 2024

Any progress?

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