Codecov Report

Patch and project coverage have no change.

Comparison is base(66aa7db) 95.52% compared to head(c9346ee) 95.52%.

@@           Coverage Diff           @@##           master     #213   +/-   ##=======================================  Coverage   95.52%   95.52%           =======================================  Files          16       16             Lines        1072     1072           =======================================  Hits         1024     1024             Misses         48       48

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

Can you be more specific? The requirements of the test code is not relevant here, what's relevant is that all cores using ArduinoCore-API support C++11 in order to remove those guards.

I thought that was a requirement for using this API package.

There are several places in the production code that already use C++11 featuresunguarded e.g.line 72 in this file that uses a delegatng constructor, asdoes CanMsg.h. Other features used areauto and "commas in enum" in Common.h, "long long" in Print.h. Possibly more, the compiler gave up at that point.

Given the guards were only in String code and that not all use of C++11 features was being guarded I thought they were old dead code that could be cleaned up.

That's true. But we do now know how many or which cores (that use ArduinoCore-API) are out there that do not support advanced language features. If anything, those advanced C++ usage should be hidden behind the same kind of include guards.

We do know that all corescurrently using ArduinoCore-API support C++11 because its features are already used, unguarded, in several places in the API code. That includes the arduinomegaavr andsamd cores.

Are there corestrying to adopt ArduinoCore-API that can't provide C++11 support?

The one that jumps to mind might be the arduinoAVR core but my understanding is that'salready using-std=gnu++11 so that wouldn't be an issue.

The issue is more about 3rd party cores, but a quick sampling of popular cores shows that Arduino_Core_STM32 usesC++17), arduino-esp32 usesC++17 and yes, even ArduinoCore-avr uses [C++11]. I therefore consider it safe to merge this issue, if a 3rd party corethat already uses ArduinoCore-API want to upgrade to the next released version they may as well consider updating their tooling. Furthermore, as you've said it, the CAN API and other code sequences in ArduinoCore-APIalready require a minimum of C++11.