FortySevenEffects / arduino_midi_library

MIDI for Arduino
MIT License
1.56k stars 252 forks source link

Replace all hardcoded "midi::" namespace by "MIDI_NAMESPACE::" #321

Open YaelBx opened 1 year ago

YaelBx commented 1 year ago

The real issue is the hardcoded one in _src/midiMessage.h when trying to edit the default "midi" namespace. IMHO, if we can use custom namespace, no midi:: namespace should be hardcoded, we should instead always use the MIDI_NAMESPACE MACRO.

I did not managed to install the unit-test to verify my modifications, too much issues with Visual and /W argument. If someone with a working unit-test config can verify the modifications, this would be lovely.

YaelBx commented 1 year ago

praise: Thanks, looks good!

@franky47 reminder, I did not managed to execute unit-test on my setup, can you ACK me that you did run them and all good? Thanks again, Yaël

franky47 commented 1 year ago

They are running in the CI on GitHub actions, let's wait and see 👀.

franky47 commented 1 year ago

The error I see comes from Google Test itself, I'll have to investigate.