Conversation
|
There's a couple of instances of "SF2/3" that have been mistakenly changed to "SoundFont/3". Edit: Literally all of these are unused Carla translations, which will be removed in #8013, so this isn't really a meaningful problem after all. |
rubiefawn
left a comment
There was a problem hiding this comment.
I have some additional non-blocking feedback. Whether you make these changes or not, I will extend approval regardless. See below.
In this PR, you touch several lines of old code that do not follow our coding conventions. Since you're updating these lines, you might as well take this opportunity to update the code style as well:
- Function parameters: Some of the function parameters begin with an underscore, e.g.
_instrument_trackshould beinstrument_track. These parameters should be renamed to remove the underscore.- There's a few function parameters named
_this; just rename these toel, sincethisis a keyword.
- There's a few function parameters named
- Braces: Extra spaces shouldn't be inside parentheses, e.g.
( thing )should be(thing). - Pointers & references: Pointer/reference specifiers (
*,&) should cozy up to the type, e.g.QDomDocument & docshould beQDomDocument& doc.
Good work!
| const QString SAMPLES_PATH = "samples/"; | ||
| const QString GIG_PATH = "samples/gig/"; | ||
| const QString SF2_PATH = "samples/soundfonts/"; | ||
| const QString SoundFont_PATH = "samples/soundfonts/"; |
There was a problem hiding this comment.
(Non-blocking) Per the Coding Conventions:
Global scope constants SHALL use
UpperCamelCase.
| const QString SoundFont_PATH = "samples/soundfonts/"; | |
| const QString SoundFontPath = "samples/soundfonts/"; |
It may look out of place next to all the other UPPER_SNAKE_CASE constants; you may update those too if you feel like doing so.
|
"SOUNDFONT" is trademarked, we probably shouldn't rename this. |
I didn't realize this, this is a pretty big problem. Should it be "sfplayer" then? What are some other options for renaming this? |
|
I'm quite caught up this week with university stuff, I'll see about getting to changing to sfplayer and implementing the code practices above this weekend. |
No description provided.