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

Fix include guards and copyright statements #6603

Merged
merged 5 commits into from
Feb 18, 2023

Conversation

messmerd
Copy link
Member

@messmerd messmerd commented Jan 5, 2023

This PR corrects some minor issues with header files under include/

  • Added namespace prefixes (LMMS_ or LMMS_GUI_) to include guards of header files in the include/ directory. This fixes a name conflict between the include guards of include/Engine.h and plugins/ZynAddSubFx/zynaddsubfx/src/Nio/Engine.h and possibly others. It will also prevent any future conflicts.
  • The include guards of Clip.h and ClipView.h were still using their old "track content object" names from before the reorganization. This has been corrected. (#5592)
  • Added missing copyright statement to MidiCCRackView.h.
  • Added missing copyright statements to CustomTextKnob.h and CustomTextKnob.cpp according to author's comment in #5321.
  • Makes the number of newlines between copyright statement and include guard, between include guard and includes, and between closing include guard and EOF consistent for all headers

Copy link
Contributor

@Veratil Veratil left a comment

Choose a reason for hiding this comment

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

LGTM

My only note is a few of the #endif's lost their // GUARD_H comment. We don't really need them since they're EOF, but it doesn't hurt to have I think?

@messmerd
Copy link
Member Author

messmerd commented Jan 5, 2023

My only note is a few of the #endif's lost their // GUARD_H comment. We don't really need them since they're EOF, but it doesn't hurt to have I think?

There were only a few instances of those, so I just removed them for consistency with the rest of the headers.

@messmerd
Copy link
Member Author

messmerd commented Jan 5, 2023

Actually, I'll go ahead and add them to all the headers. It would be better to have them, and now's the best time to do it.

@PhysSong
Copy link
Member

PhysSong commented Jan 6, 2023

Generally looks good.
I'm not sure if it's necessary, but I think it's a good chance to rename some include guards, ex. LMMS_AUDIO_DEVICE_H to LMMS_AUDIODEVICE_H. Historically, some header files used names in snake_case, but they were renamed to PascalCase.

include/Editor.h Outdated Show resolved Hide resolved
include/Lv2ViewBase.h Outdated Show resolved Hide resolved
include/aeffectx.h Outdated Show resolved Hide resolved
@Veratil
Copy link
Contributor

Veratil commented Feb 18, 2023

Since @PhysSong has soft approval with comment, and @DomClark has approval. I'm merging this now.

@Veratil Veratil merged commit 534e7ed into LMMS:master Feb 18, 2023
@messmerd messmerd deleted the include-guard-fix branch February 19, 2023 00:00
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.

4 participants