search

approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
310 stars 50 forks source link

#213 Fix bug with {fmt} integration #214

Open YarikTH opened 9 months ago

YarikTH commented 9 months ago

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 ( llewellyn.falco@gmail.com ) Usually the sessions are between 45-90 minutes.

assuming you still feel it is small, please include

Description

See linked issue https://github.com/approvals/ApprovalTests.cpp/issues/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 commented 9 months ago

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

claremacrae commented 9 months ago

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 commented 3 months ago

Any progress?