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

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

Open
wants to merge 1 commit into
base: feat/v5.1.0
Choose a base branch
from

Conversation

YaelBx
Copy link

@YaelBx YaelBx commented Feb 17, 2023

The real issue is the hardcoded one in src/midi_Message.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.

Signed-off-by: Yaël Boutreux <yael.boutreux@gmail.com>
Copy link
Member

@franky47 franky47 left a comment

Choose a reason for hiding this comment

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

praise: Thanks, looks good!

@YaelBx
Copy link
Author

YaelBx commented Feb 17, 2023

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
Copy link
Member

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

@franky47
Copy link
Member

franky47 commented Feb 17, 2023

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

@franky47 franky47 changed the base branch from master to feat/v5.1.0 July 18, 2023 08:30
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